Skip to content

Commit 311b0fd

Browse files
SamMorrowDrumsSamMorrowDrums
committed
Address review feedback: enum size validation, mutation fix, tests
- Replace runtime size validation with compile-time enum type (Size with SizeSM=16, SizeLG=24) - Fix RegisterFunc mutation by making shallow copy of tool before modifying Icons - Add comprehensive tests for octicons package (URL, Icons, Size constants) - Add toolsets tests for ToolsetMetadata.Icons(), RegisterFunc mutation prevention, and existing icon preservation - Improve icon choices for better visual semantics: - actions: play → workflow (more specific to GitHub Actions) - secret_protection: key → shield-lock (better represents protection) - gists: code → logo-gist (dedicated gist icon exists)
1 parent 16a0162 commit 311b0fd

13 files changed

Lines changed: 416 additions & 32 deletions

‎pkg/github/__toolsnaps__/assign_copilot_to_issue.snap‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,21 @@
2626
}
2727
}
2828
},
29-
"name": "assign_copilot_to_issue"
29+
"name": "assign_copilot_to_issue",
30+
"icons": [
31+
{
32+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/copilot-16.svg",
33+
"mimeType": "image/svg+xml",
34+
"sizes": [
35+
"16x16"
36+
]
37+
},
38+
{
39+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/copilot-24.svg",
40+
"mimeType": "image/svg+xml",
41+
"sizes": [
42+
"24x24"
43+
]
44+
}
45+
]
3046
}

‎pkg/github/__toolsnaps__/fork_repository.snap‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,21 @@
2424
}
2525
}
2626
},
27-
"name": "fork_repository"
27+
"name": "fork_repository",
28+
"icons": [
29+
{
30+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/repo-forked-16.svg",
31+
"mimeType": "image/svg+xml",
32+
"sizes": [
33+
"16x16"
34+
]
35+
},
36+
{
37+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/repo-forked-24.svg",
38+
"mimeType": "image/svg+xml",
39+
"sizes": [
40+
"24x24"
41+
]
42+
}
43+
]
2844
}

‎pkg/github/__toolsnaps__/merge_pull_request.snap‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,21 @@
4242
}
4343
}
4444
},
45-
"name": "merge_pull_request"
45+
"name": "merge_pull_request",
46+
"icons": [
47+
{
48+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/git-merge-16.svg",
49+
"mimeType": "image/svg+xml",
50+
"sizes": [
51+
"16x16"
52+
]
53+
},
54+
{
55+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/git-merge-24.svg",
56+
"mimeType": "image/svg+xml",
57+
"sizes": [
58+
"24x24"
59+
]
60+
}
61+
]
4662
}

‎pkg/github/__toolsnaps__/request_copilot_review.snap‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,21 @@
2525
}
2626
}
2727
},
28-
"name": "request_copilot_review"
28+
"name": "request_copilot_review",
29+
"icons": [
30+
{
31+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/copilot-16.svg",
32+
"mimeType": "image/svg+xml",
33+
"sizes": [
34+
"16x16"
35+
]
36+
},
37+
{
38+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/copilot-24.svg",
39+
"mimeType": "image/svg+xml",
40+
"sizes": [
41+
"24x24"
42+
]
43+
}
44+
]
2945
}

‎pkg/github/__toolsnaps__/star_repository.snap‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,21 @@
2020
}
2121
}
2222
},
23-
"name": "star_repository"
23+
"name": "star_repository",
24+
"icons": [
25+
{
26+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/star-fill-16.svg",
27+
"mimeType": "image/svg+xml",
28+
"sizes": [
29+
"16x16"
30+
]
31+
},
32+
{
33+
"src": "https://raw.githubusercontent.com/primer/octicons/main/icons/star-fill-24.svg",
34+
"mimeType": "image/svg+xml",
35+
"sizes": [
36+
"24x24"
37+
]
38+
}
39+
]
2440
}

‎pkg/github/issues.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
ghErrors "github.com/github/github-mcp-server/pkg/errors"
1313
"github.com/github/github-mcp-server/pkg/lockdown"
14+
"github.com/github/github-mcp-server/pkg/octicons"
1415
"github.com/github/github-mcp-server/pkg/sanitize"
1516
"github.com/github/github-mcp-server/pkg/toolsets"
1617
"github.com/github/github-mcp-server/pkg/translations"
@@ -1622,6 +1623,7 @@ func AssignCopilotToIssue(t translations.TranslationHelperFunc) toolsets.ServerT
16221623
mcp.Tool{
16231624
Name: "assign_copilot_to_issue",
16241625
Description: t("TOOL_ASSIGN_COPILOT_TO_ISSUE_DESCRIPTION", description.String()),
1626+
Icons: octicons.Icons("copilot"),
16251627
Annotations: &mcp.ToolAnnotations{
16261628
Title: t("TOOL_ASSIGN_COPILOT_TO_ISSUE_USER_TITLE", "Assign Copilot to issue"),
16271629
ReadOnlyHint: false,

‎pkg/github/pullrequests.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
ghErrors "github.com/github/github-mcp-server/pkg/errors"
1717
"github.com/github/github-mcp-server/pkg/lockdown"
18+
"github.com/github/github-mcp-server/pkg/octicons"
1819
"github.com/github/github-mcp-server/pkg/sanitize"
1920
"github.com/github/github-mcp-server/pkg/toolsets"
2021
"github.com/github/github-mcp-server/pkg/translations"
@@ -1008,6 +1009,7 @@ func MergePullRequest(t translations.TranslationHelperFunc) toolsets.ServerTool
10081009
mcp.Tool{
10091010
Name: "merge_pull_request",
10101011
Description: t("TOOL_MERGE_PULL_REQUEST_DESCRIPTION", "Merge a pull request in a GitHub repository."),
1012+
Icons: octicons.Icons("git-merge"),
10111013
Annotations: &mcp.ToolAnnotations{
10121014
Title: t("TOOL_MERGE_PULL_REQUEST_USER_TITLE", "Merge pull request"),
10131015
ReadOnlyHint: false,
@@ -1777,6 +1779,7 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) toolsets.ServerT
17771779
mcp.Tool{
17781780
Name: "request_copilot_review",
17791781
Description: t("TOOL_REQUEST_COPILOT_REVIEW_DESCRIPTION", "Request a GitHub Copilot code review for a pull request. Use this for automated feedback on pull requests, usually before requesting a human reviewer."),
1782+
Icons: octicons.Icons("copilot"),
17801783
Annotations: &mcp.ToolAnnotations{
17811784
Title: t("TOOL_REQUEST_COPILOT_REVIEW_USER_TITLE", "Request Copilot review"),
17821785
ReadOnlyHint: false,

‎pkg/github/repositories.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111

1212
ghErrors "github.com/github/github-mcp-server/pkg/errors"
13+
"github.com/github/github-mcp-server/pkg/octicons"
1314
"github.com/github/github-mcp-server/pkg/raw"
1415
"github.com/github/github-mcp-server/pkg/toolsets"
1516
"github.com/github/github-mcp-server/pkg/translations"
@@ -776,6 +777,7 @@ func ForkRepository(t translations.TranslationHelperFunc) toolsets.ServerTool {
776777
mcp.Tool{
777778
Name: "fork_repository",
778779
Description: t("TOOL_FORK_REPOSITORY_DESCRIPTION", "Fork a GitHub repository to your account or specified organization"),
780+
Icons: octicons.Icons("repo-forked"),
779781
Annotations: &mcp.ToolAnnotations{
780782
Title: t("TOOL_FORK_REPOSITORY_USER_TITLE", "Fork repository"),
781783
ReadOnlyHint: false,
@@ -2028,6 +2030,7 @@ func StarRepository(t translations.TranslationHelperFunc) toolsets.ServerTool {
20282030
mcp.Tool{
20292031
Name: "star_repository",
20302032
Description: t("TOOL_STAR_REPOSITORY_DESCRIPTION", "Star a GitHub repository"),
2033+
Icons: octicons.Icons("star-fill"),
20312034
Annotations: &mcp.ToolAnnotations{
20322035
Title: t("TOOL_STAR_REPOSITORY_USER_TITLE", "Star repository"),
20332036
ReadOnlyHint: false,

‎pkg/github/tools.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ var (
6666
ToolsetMetadataActions = toolsets.ToolsetMetadata{
6767
ID: "actions",
6868
Description: "GitHub Actions workflows and CI/CD operations",
69-
Icon: "play",
69+
Icon: "workflow",
7070
}
7171
ToolsetMetadataCodeSecurity = toolsets.ToolsetMetadata{
7272
ID: "code_security",
@@ -76,7 +76,7 @@ var (
7676
ToolsetMetadataSecretProtection = toolsets.ToolsetMetadata{
7777
ID: "secret_protection",
7878
Description: "Secret protection related tools, such as GitHub Secret Scanning",
79-
Icon: "key",
79+
Icon: "shield-lock",
8080
}
8181
ToolsetMetadataDependabot = toolsets.ToolsetMetadata{
8282
ID: "dependabot",
@@ -101,7 +101,7 @@ var (
101101
ToolsetMetadataGists = toolsets.ToolsetMetadata{
102102
ID: "gists",
103103
Description: "GitHub Gist related tools",
104-
Icon: "code",
104+
Icon: "logo-gist",
105105
}
106106
ToolsetMetadataSecurityAdvisories = toolsets.ToolsetMetadata{
107107
ID: "security_advisories",

‎pkg/octicons/octicons.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Package octicons provides helpers for working with GitHub Octicon icons.
2+
// See https://primer.style/foundations/icons for available icons.
3+
package octicons
4+
5+
import (
6+
"fmt"
7+
8+
"github.com/modelcontextprotocol/go-sdk/mcp"
9+
)
10+
11+
// Size represents the size of an Octicon icon.
12+
type Size int
13+
14+
const (
15+
// SizeSM is the small (16x16) icon size.
16+
SizeSM Size = 16
17+
// SizeLG is the large (24x24) icon size.
18+
SizeLG Size = 24
19+
)
20+
21+
// URL returns the CDN URL for a GitHub Octicon SVG.
22+
func URL(name string, size Size) string {
23+
return fmt.Sprintf("https://raw.githubusercontent.com/primer/octicons/main/icons/%s-%d.svg", name, size)
24+
}
25+
26+
// Icons returns MCP Icon objects for the given octicon name in both 16x16 and 24x24 sizes.
27+
// Use this to set custom icons on individual tools that should override their toolset's default icon.
28+
// The name should be the base octicon name without size suffix (e.g., "copilot" not "copilot-16").
29+
// See https://primer.style/foundations/icons for available icons.
30+
func Icons(name string) []mcp.Icon {
31+
if name == "" {
32+
return nil
33+
}
34+
return []mcp.Icon{
35+
{
36+
Source: URL(name, SizeSM),
37+
MIMEType: "image/svg+xml",
38+
Sizes: []string{"16x16"},
39+
},
40+
{
41+
Source: URL(name, SizeLG),
42+
MIMEType: "image/svg+xml",
43+
Sizes: []string{"24x24"},
44+
},
45+
}
46+
}

0 commit comments

Comments
 (0)