Skip to content

Commit 2d83436

Browse files
tonytrgtonytrg
committed
change functions
1 parent 5346e05 commit 2d83436

File tree

3 files changed

+135
-76
lines changed

3 files changed

+135
-76
lines changed

‎internal/ghmcp/server.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
120120
if github.ContainsToolset(enabledToolsets, github.ToolsetMetadataAll.ID) {
121121
enabledToolsets = []string{github.ToolsetMetadataAll.ID}
122122
}
123+
if github.ContainsToolset(enabledToolsets, github.ToolsetMetadataDefault.ID) {
124+
enabledToolsets = github.AddDefaultToolset(enabledToolsets)
125+
}
123126

124127
if len(invalidToolsets) > 0 {
125128
fmt.Fprintf(os.Stderr, "Invalid toolsets ignored: %s\n", strings.Join(invalidToolsets, ", "))

‎pkg/github/tools.go‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -446,11 +446,36 @@ func GenerateToolsetsHelp() string {
446446
return toolsetsHelp
447447
}
448448

449+
// AddDefaultToolset removes the default toolset and expands it to the actual default toolset IDs
450+
func AddDefaultToolset(result []string) []string {
451+
hasDefault := false
452+
seen := make(map[string]bool)
453+
for _, toolset := range result {
454+
seen[toolset] = true
455+
if toolset == ToolsetMetadataDefault.ID {
456+
hasDefault = true
457+
}
458+
}
459+
460+
// Only expand if "default" keyword was found
461+
if !hasDefault {
462+
return result
463+
}
464+
465+
result = RemoveToolset(result, ToolsetMetadataDefault.ID)
466+
467+
for _, defaultToolset := range GetDefaultToolsetIDs() {
468+
if !seen[defaultToolset] {
469+
result = append(result, defaultToolset)
470+
}
471+
}
472+
return result
473+
}
474+
449475
// cleanToolsets cleans and handles special toolset keywords:
450476
// - Duplicates are removed from the result
451477
// - Removes whitespaces
452-
// - Validates toolset names and returns invalid ones separately
453-
// - "default": Replaces with the actual default toolset IDs from GetDefaultToolsetIDs()
478+
// - Validates toolset names and returns invalid ones separately - for warning reporting
454479
// Returns: (toolsets, invalidToolsets)
455480
func CleanToolsets(enabledToolsets []string) ([]string, []string) {
456481
seen := make(map[string]bool)
@@ -466,24 +491,9 @@ func CleanToolsets(enabledToolsets []string) ([]string, []string) {
466491
}
467492
if !seen[trimmed] {
468493
seen[trimmed] = true
469-
if trimmed != ToolsetMetadataDefault.ID && trimmed != ToolsetMetadataAll.ID {
470-
// Validate the toolset name
471-
result = append(result, trimmed)
472-
if !validIDs[trimmed] {
473-
invalid = append(invalid, trimmed)
474-
}
475-
}
476-
}
477-
}
478-
479-
hasDefault := seen[ToolsetMetadataDefault.ID]
480-
481-
// Expand "default" keyword to actual default toolsets
482-
if hasDefault {
483-
for _, defaultToolset := range GetDefaultToolsetIDs() {
484-
if !seen[defaultToolset] {
485-
result = append(result, defaultToolset)
486-
seen[defaultToolset] = true
494+
result = append(result, trimmed)
495+
if !validIDs[trimmed] {
496+
invalid = append(invalid, trimmed)
487497
}
488498
}
489499
}

‎pkg/github/tools_test.go‎

Lines changed: 102 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -24,64 +24,41 @@ func TestCleanToolsets(t *testing.T) {
2424
input: nil,
2525
expected: []string{},
2626
},
27-
// default test cases
27+
// CleanToolsets only cleans - it does NOT filter out special keywords
2828
{
29-
name: "default only",
30-
input: []string{"default"},
31-
expected: []string{
32-
"context",
33-
"repos",
34-
"issues",
35-
"pull_requests",
36-
"users",
37-
},
29+
name: "default keyword preserved",
30+
input: []string{"default"},
31+
expected: []string{"default"},
3832
},
3933
{
40-
name: "default with additional toolsets",
41-
input: []string{"default", "actions", "gists"},
42-
expected: []string{
43-
"actions",
44-
"gists",
45-
"context",
46-
"repos",
47-
"issues",
48-
"pull_requests",
49-
"users",
50-
},
34+
name: "default with additional toolsets",
35+
input: []string{"default", "actions", "gists"},
36+
expected: []string{"default", "actions", "gists"},
37+
},
38+
{
39+
name: "all keyword preserved",
40+
input: []string{"all", "actions"},
41+
expected: []string{"all", "actions"},
5142
},
5243
{
53-
name: "no default present",
44+
name: "no special keywords",
5445
input: []string{"actions", "gists", "notifications"},
5546
expected: []string{"actions", "gists", "notifications"},
5647
},
5748
{
58-
name: "duplicate toolsets without default",
49+
name: "duplicate toolsets without special keywords",
5950
input: []string{"actions", "gists", "actions"},
6051
expected: []string{"actions", "gists"},
6152
},
6253
{
63-
name: "duplicate toolsets with default",
64-
input: []string{"context", "repos", "issues", "pull_requests", "users", "default"},
65-
expected: []string{
66-
"context",
67-
"repos",
68-
"issues",
69-
"pull_requests",
70-
"users",
71-
},
54+
name: "duplicate toolsets with default",
55+
input: []string{"context", "repos", "issues", "pull_requests", "users", "default"},
56+
expected: []string{"context", "repos", "issues", "pull_requests", "users", "default"},
7257
},
7358
{
74-
name: "default appears multiple times with different toolsets in between",
75-
input: []string{"default", "actions", "default", "gists", "default"},
76-
expected: []string{
77-
"actions",
78-
"gists",
79-
"context",
80-
"repos",
81-
"issues",
82-
"pull_requests",
83-
"users",
84-
},
59+
name: "default appears multiple times - duplicates removed",
60+
input: []string{"default", "actions", "default", "gists", "default"},
61+
expected: []string{"default", "actions", "gists"},
8562
},
8663
// Whitespace test cases
8764
{
@@ -90,17 +67,14 @@ func TestCleanToolsets(t *testing.T) {
9067
expected: []string{"actions", "gists", "notifications"},
9168
},
9269
{
93-
name: "whitespace check - default toolset",
94-
input: []string{" actions ", " default ", "notifications"},
95-
expected: []string{
96-
"actions",
97-
"notifications",
98-
"context",
99-
"repos",
100-
"issues",
101-
"pull_requests",
102-
"users",
103-
},
70+
name: "whitespace check - default toolset with whitespace",
71+
input: []string{" actions ", " default ", "notifications"},
72+
expected: []string{"actions", "default", "notifications"},
73+
},
74+
{
75+
name: "whitespace check - all toolset with whitespace",
76+
input: []string{" all ", " actions "},
77+
expected: []string{"all", "actions"},
10478
},
10579
// Invalid toolset test cases
10680
{
@@ -158,9 +132,81 @@ func TestCleanToolsets(t *testing.T) {
158132
assert.Equal(t, expectedInvalidMap, invalidMap, "invalid should contain all expected invalid toolsets")
159133

160134
assert.Len(t, resultMap, len(result), "result should not contain duplicates")
135+
})
136+
}
137+
}
138+
139+
func TestAddDefaultToolset(t *testing.T) {
140+
tests := []struct {
141+
name string
142+
input []string
143+
expected []string
144+
}{
145+
{
146+
name: "no default keyword - return unchanged",
147+
input: []string{"actions", "gists"},
148+
expected: []string{"actions", "gists"},
149+
},
150+
{
151+
name: "default keyword present - expand and remove default",
152+
input: []string{"default"},
153+
expected: []string{
154+
"context",
155+
"repos",
156+
"issues",
157+
"pull_requests",
158+
"users",
159+
},
160+
},
161+
{
162+
name: "default with additional toolsets",
163+
input: []string{"default", "actions", "gists"},
164+
expected: []string{
165+
"actions",
166+
"gists",
167+
"context",
168+
"repos",
169+
"issues",
170+
"pull_requests",
171+
"users",
172+
},
173+
},
174+
{
175+
name: "default with overlapping toolsets - should not duplicate",
176+
input: []string{"default", "context", "repos"},
177+
expected: []string{
178+
"context",
179+
"repos",
180+
"issues",
181+
"pull_requests",
182+
"users",
183+
},
184+
},
185+
{
186+
name: "empty input",
187+
input: []string{},
188+
expected: []string{},
189+
},
190+
}
191+
192+
for _, tt := range tests {
193+
t.Run(tt.name, func(t *testing.T) {
194+
result := AddDefaultToolset(tt.input)
195+
196+
require.Len(t, result, len(tt.expected), "result length should match expected length")
197+
198+
resultMap := make(map[string]bool)
199+
for _, toolset := range result {
200+
resultMap[toolset] = true
201+
}
202+
203+
expectedMap := make(map[string]bool)
204+
for _, toolset := range tt.expected {
205+
expectedMap[toolset] = true
206+
}
161207

162-
assert.False(t, resultMap["default"], "result should not contain 'default'")
163-
assert.False(t, resultMap["all"], "result should not contain 'all'")
208+
assert.Equal(t, expectedMap, resultMap, "result should contain all expected toolsets")
209+
assert.False(t, resultMap["default"], "result should not contain 'default' keyword")
164210
})
165211
}
166212
}

0 commit comments

Comments
 (0)