Skip to content

Commit 30a62d4

Browse files
refactor: add NewStandardBuilder for consistent inventory building
Add github.NewStandardBuilder() with InventoryConfig to provide a single canonical way to build inventories with all standard filters: - DeprecatedAliases - ReadOnly - Toolsets - Tools - FeatureChecker This replaces scattered manual builder chains across: - cmd/github-mcp-server/main.go (OAuth scope computation) - cmd/github-mcp-server/list_scopes.go (CLI command) - internal/ghmcp/server.go (server initialization) Benefits: - Single point to add new filters (no scattered updates) - main.go was missing WithDeprecatedAliases - now fixed - list_scopes.go was missing features flag support - now fixed - Consistent behavior across all inventory usage
1 parent c2a68df commit 30a62d4

File tree

4 files changed

+62
-33
lines changed

4 files changed

+62
-33
lines changed

cmd/github-mcp-server/list_scopes.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,27 +101,28 @@ func runListScopes() error {
101101
}
102102
}
103103

104+
// Get enabled features (similar to toolsets)
105+
var enabledFeatures []string
106+
if viper.IsSet("features") {
107+
if err := viper.UnmarshalKey("features", &enabledFeatures); err != nil {
108+
return fmt.Errorf("failed to unmarshal features: %w", err)
109+
}
110+
}
111+
104112
readOnly := viper.GetBool("read-only")
105113
outputFormat := viper.GetString("list-scopes-output")
106114

107115
// Create translation helper
108116
t, _ := translations.TranslationHelper()
109117

110-
// Build inventory using the same logic as the stdio server
111-
inventoryBuilder := github.NewInventory(t).
112-
WithReadOnly(readOnly)
113-
114-
// Configure toolsets (same as stdio)
115-
if enabledToolsets != nil {
116-
inventoryBuilder = inventoryBuilder.WithToolsets(enabledToolsets)
117-
}
118-
119-
// Configure specific tools
120-
if len(enabledTools) > 0 {
121-
inventoryBuilder = inventoryBuilder.WithTools(enabledTools)
122-
}
123-
124-
inv, err := inventoryBuilder.Build()
118+
// Build inventory using the shared builder for consistency
119+
inv, err := github.NewStandardBuilder(github.InventoryConfig{
120+
Translator: t,
121+
ReadOnly: readOnly,
122+
Toolsets: enabledToolsets,
123+
Tools: enabledTools,
124+
EnabledFeatures: enabledFeatures,
125+
}).Build()
125126
if err != nil {
126127
return fmt.Errorf("failed to build inventory: %w", err)
127128
}

cmd/github-mcp-server/main.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,13 @@ func getOAuthScopes(enabledToolsets, enabledTools, enabledFeatures []string, t t
230230
// Build inventory with the same configuration that will be used at runtime
231231
// This allows us to determine which tools will actually be available
232232
// and avoids building the inventory twice
233-
inventoryBuilder := github.NewInventory(t).
234-
WithReadOnly(viper.GetBool("read-only")).
235-
WithToolsets(enabledToolsets).
236-
WithTools(enabledTools).
237-
WithFeatureChecker(inventory.NewSliceFeatureChecker(enabledFeatures))
233+
inventoryBuilder := github.NewStandardBuilder(github.InventoryConfig{
234+
Translator: t,
235+
ReadOnly: viper.GetBool("read-only"),
236+
Toolsets: enabledToolsets,
237+
Tools: enabledTools,
238+
EnabledFeatures: enabledFeatures,
239+
})
238240

239241
inv, err := inventoryBuilder.Build()
240242
if err != nil {

internal/ghmcp/server.go

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -258,13 +258,13 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
258258
// Prebuilt inventory from OAuth scope computation doesn't have scope filter yet
259259
if cfg.TokenScopes != nil {
260260
// Need to rebuild with scope filter
261-
inventoryBuilder := github.NewInventory(cfg.Translator).
262-
WithDeprecatedAliases(github.DeprecatedToolAliases).
263-
WithReadOnly(cfg.ReadOnly).
264-
WithToolsets(enabledToolsets).
265-
WithTools(cfg.EnabledTools).
266-
WithFeatureChecker(featureChecker).
267-
WithFilter(github.CreateToolScopeFilter(cfg.TokenScopes))
261+
inventoryBuilder := github.NewStandardBuilder(github.InventoryConfig{
262+
Translator: cfg.Translator,
263+
ReadOnly: cfg.ReadOnly,
264+
Toolsets: enabledToolsets,
265+
Tools: cfg.EnabledTools,
266+
EnabledFeatures: cfg.EnabledFeatures,
267+
}).WithFilter(github.CreateToolScopeFilter(cfg.TokenScopes))
268268

269269
var err error
270270
inv, err = inventoryBuilder.Build()
@@ -274,12 +274,13 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
274274
}
275275
} else {
276276
// Build inventory from scratch
277-
inventoryBuilder := github.NewInventory(cfg.Translator).
278-
WithDeprecatedAliases(github.DeprecatedToolAliases).
279-
WithReadOnly(cfg.ReadOnly).
280-
WithToolsets(enabledToolsets).
281-
WithTools(cfg.EnabledTools).
282-
WithFeatureChecker(featureChecker)
277+
inventoryBuilder := github.NewStandardBuilder(github.InventoryConfig{
278+
Translator: cfg.Translator,
279+
ReadOnly: cfg.ReadOnly,
280+
Toolsets: enabledToolsets,
281+
Tools: cfg.EnabledTools,
282+
EnabledFeatures: cfg.EnabledFeatures,
283+
})
283284

284285
// Apply token scope filtering if scopes are known (for PAT filtering)
285286
if cfg.TokenScopes != nil {

pkg/github/inventory.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,28 @@ func NewInventory(t translations.TranslationHelperFunc) *inventory.Builder {
1616
SetResources(AllResources(t)).
1717
SetPrompts(AllPrompts(t))
1818
}
19+
20+
// InventoryConfig holds configuration for building an inventory with standard filters.
21+
// This struct enables consistent inventory building across the codebase.
22+
type InventoryConfig struct {
23+
Translator translations.TranslationHelperFunc
24+
ReadOnly bool
25+
Toolsets []string // nil = use defaults, empty = none
26+
Tools []string // additional specific tools
27+
EnabledFeatures []string // feature flags
28+
}
29+
30+
// NewStandardBuilder creates an inventory builder with all standard filters applied.
31+
// This is the canonical way to create an inventory builder, ensuring consistency
32+
// between OAuth scope computation, server initialization, and CLI tools.
33+
//
34+
// The returned builder can be further customized (e.g., WithFilter for scope filtering)
35+
// before calling Build().
36+
func NewStandardBuilder(cfg InventoryConfig) *inventory.Builder {
37+
return NewInventory(cfg.Translator).
38+
WithDeprecatedAliases(DeprecatedToolAliases).
39+
WithReadOnly(cfg.ReadOnly).
40+
WithToolsets(cfg.Toolsets).
41+
WithTools(cfg.Tools).
42+
WithFeatureChecker(inventory.NewSliceFeatureChecker(cfg.EnabledFeatures))
43+
}

0 commit comments

Comments
 (0)