Skip to content

Commit 03d2f16

Browse files
Add icon validation tests and single source of truth for required icons
- Add pkg/octicons/required_icons.txt as single source of truth for icons - Add RequiredIcons() function to read the required icons list - Update script/fetch-icons to read from required_icons.txt - Update octicons_test.go to use RequiredIcons() instead of hardcoded list - Add pkg/github/toolset_icons_test.go with: - TestAllToolsetIconsExist: validates all toolset icons are embedded - TestToolsetMetadataHasIcons: ensures all toolsets have icons set - Add 'book' icon for SupportSearch toolset - Update docs/toolsets-and-icons.md with fetch-icons and CI validation docs
1 parent 54fac00 commit 03d2f16

File tree

8 files changed

+223
-60
lines changed

8 files changed

+223
-60
lines changed

docs/toolsets-and-icons.md

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,42 @@ Icons help users quickly identify toolsets in MCP-compatible clients. We use [Pr
3232

3333
Browse the [Octicon gallery](https://primer.style/foundations/icons) and select an appropriate icon. Use the base name without size suffix (e.g., `repo` not `repo-16`).
3434

35-
### Step 2: Add the Icon Files
35+
### Step 2: Add Icon to Required Icons List
3636

37-
Icons are stored as PNG files in `pkg/octicons/icons/` with light and dark theme variants:
37+
Icons are defined in `pkg/octicons/required_icons.txt`, which is the single source of truth for which icons should be embedded:
3838

3939
```
40-
pkg/octicons/icons/
41-
├── repo-light.png # For light theme
42-
├── repo-dark.png # For dark theme
43-
├── issue-opened-light.png
44-
├── issue-opened-dark.png
45-
└── ...
40+
# Required icons for the GitHub MCP Server
41+
# Add new icons below (one per line)
42+
repo
43+
issue-opened
44+
git-pull-request
45+
your-new-icon # Add your icon here
4646
```
4747

48-
Icon files should be 20x20 pixels in size.
48+
### Step 3: Fetch the Icon Files
4949

50-
### Step 3: Update the Toolset Metadata
50+
Run the fetch-icons script to download and convert the icon:
51+
52+
```bash
53+
# Fetch a specific icon
54+
script/fetch-icons your-new-icon
55+
56+
# Or fetch all required icons
57+
script/fetch-icons
58+
```
59+
60+
This script:
61+
- Downloads the 24px SVG from [Primer Octicons](https://github.com/primer/octicons)
62+
- Converts to PNG with light theme (dark icons for light backgrounds)
63+
- Converts to PNG with dark theme (white icons for dark backgrounds)
64+
- Saves both variants to `pkg/octicons/icons/`
65+
66+
**Requirements:** The script requires `rsvg-convert`:
67+
- Ubuntu/Debian: `sudo apt-get install librsvg2-bin`
68+
- macOS: `brew install librsvg`
69+
70+
### Step 4: Update the Toolset Metadata
5171

5272
Add or update the `Icon` field in the toolset definition:
5373

@@ -61,7 +81,7 @@ ToolsetMetadataRepos = inventory.ToolsetMetadata{
6181
}
6282
```
6383

64-
### Step 4: Regenerate Documentation
84+
### Step 5: Regenerate Documentation
6585

6686
Run the documentation generator to update all markdown files:
6787

@@ -158,3 +178,24 @@ icons := octicons.Icons("repo")
158178
1. Verify the client supports MCP tool icons
159179
2. Check that the octicons package is properly generating base64 data URIs
160180
3. Ensure the icon name matches a file in `pkg/octicons/icons/`
181+
182+
## CI Validation
183+
184+
The following tests run in CI to catch icon issues early:
185+
186+
### `pkg/octicons.TestEmbeddedIconsExist`
187+
188+
Verifies that all icons listed in `pkg/octicons/required_icons.txt` have corresponding PNG files embedded.
189+
190+
### `pkg/github.TestAllToolsetIconsExist`
191+
192+
Verifies that all toolset `Icon` fields reference icons that are properly embedded.
193+
194+
### `pkg/github.TestToolsetMetadataHasIcons`
195+
196+
Ensures all toolsets have an `Icon` field set.
197+
198+
If any of these tests fail:
199+
1. Add the missing icon to `pkg/octicons/required_icons.txt`
200+
2. Run `script/fetch-icons` to download the icon
201+
3. Commit the new icon files

pkg/github/toolset_icons_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package github
2+
3+
import (
4+
"testing"
5+
6+
"github.com/github/github-mcp-server/pkg/octicons"
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// TestAllToolsetIconsExist validates that every toolset with an Icon field
12+
// references an icon that actually exists in the embedded octicons.
13+
// This prevents broken icon references from being merged.
14+
func TestAllToolsetIconsExist(t *testing.T) {
15+
// Get all available toolsets from the inventory
16+
inv := NewInventory(stubTranslator).Build()
17+
toolsets := inv.AvailableToolsets()
18+
19+
// Also test remote-only toolsets
20+
remoteToolsets := RemoteOnlyToolsets()
21+
22+
// Combine both lists
23+
allToolsets := make([]struct {
24+
name string
25+
icon string
26+
}, 0)
27+
28+
for _, ts := range toolsets {
29+
if ts.Icon != "" {
30+
allToolsets = append(allToolsets, struct {
31+
name string
32+
icon string
33+
}{name: string(ts.ID), icon: ts.Icon})
34+
}
35+
}
36+
37+
for _, ts := range remoteToolsets {
38+
if ts.Icon != "" {
39+
allToolsets = append(allToolsets, struct {
40+
name string
41+
icon string
42+
}{name: string(ts.ID), icon: ts.Icon})
43+
}
44+
}
45+
46+
require.NotEmpty(t, allToolsets, "expected at least one toolset with an icon")
47+
48+
for _, ts := range allToolsets {
49+
t.Run(ts.name, func(t *testing.T) {
50+
// Check that icons return valid data URIs (not empty)
51+
icons := octicons.Icons(ts.icon)
52+
require.NotNil(t, icons, "toolset %s references icon %q which does not exist", ts.name, ts.icon)
53+
assert.Len(t, icons, 2, "expected light and dark icon variants for toolset %s", ts.name)
54+
55+
// Verify both variants have valid data URIs
56+
for _, icon := range icons {
57+
assert.NotEmpty(t, icon.Source, "icon source should not be empty for toolset %s", ts.name)
58+
assert.Contains(t, icon.Source, "data:image/png;base64,",
59+
"icon %s for toolset %s should be a valid data URI", ts.icon, ts.name)
60+
}
61+
})
62+
}
63+
}
64+
65+
// TestToolsetMetadataHasIcons ensures all toolsets have icons defined.
66+
// This is a policy test - if you want to allow toolsets without icons,
67+
// you can remove or modify this test.
68+
func TestToolsetMetadataHasIcons(t *testing.T) {
69+
// These toolsets are expected to NOT have icons (internal/special purpose)
70+
exceptionsWithoutIcons := map[string]bool{
71+
"all": true, // Meta-toolset
72+
"default": true, // Meta-toolset
73+
}
74+
75+
inv := NewInventory(stubTranslator).Build()
76+
toolsets := inv.AvailableToolsets()
77+
78+
for _, ts := range toolsets {
79+
if exceptionsWithoutIcons[string(ts.ID)] {
80+
continue
81+
}
82+
t.Run(string(ts.ID), func(t *testing.T) {
83+
assert.NotEmpty(t, ts.Icon, "toolset %s should have an icon defined", ts.ID)
84+
})
85+
}
86+
}

pkg/octicons/icons/book-dark.png

318 Bytes
Loading

pkg/octicons/icons/book-light.png

395 Bytes
Loading

pkg/octicons/octicons.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,37 @@
33
package octicons
44

55
import (
6+
"bufio"
67
"embed"
78
"encoding/base64"
89
"fmt"
10+
"strings"
911

1012
"github.com/modelcontextprotocol/go-sdk/mcp"
1113
)
1214

1315
//go:embed icons/*.png
1416
var iconsFS embed.FS
1517

18+
//go:embed required_icons.txt
19+
var requiredIconsTxt string
20+
21+
// RequiredIcons returns the list of icon names from required_icons.txt.
22+
// This is the single source of truth for which icons should be embedded.
23+
func RequiredIcons() []string {
24+
var icons []string
25+
scanner := bufio.NewScanner(strings.NewReader(requiredIconsTxt))
26+
for scanner.Scan() {
27+
line := strings.TrimSpace(scanner.Text())
28+
// Skip empty lines and comments
29+
if line == "" || strings.HasPrefix(line, "#") {
30+
continue
31+
}
32+
icons = append(icons, line)
33+
}
34+
return icons
35+
}
36+
1637
// Theme represents the color theme of an icon.
1738
type Theme string
1839

pkg/octicons/octicons_test.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,9 @@ func TestThemeConstants(t *testing.T) {
104104
}
105105

106106
func TestEmbeddedIconsExist(t *testing.T) {
107-
// Test that all icons used by toolsets are properly embedded
108-
expectedIcons := []string{
109-
"apps", "beaker", "bell", "check-circle", "codescan",
110-
"comment-discussion", "copilot", "dependabot", "file", "git-branch",
111-
"git-commit", "git-merge", "git-pull-request", "issue-opened", "logo-gist", "mark-github",
112-
"organization", "people", "person", "project", "repo", "repo-forked",
113-
"shield", "shield-lock", "star", "star-fill", "tag", "tools", "workflow",
114-
}
115-
107+
// Test that all required icons from required_icons.txt are properly embedded
108+
// This is the single source of truth for which icons should be available
109+
expectedIcons := RequiredIcons()
116110
for _, icon := range expectedIcons {
117111
t.Run(icon, func(t *testing.T) {
118112
lightURI := DataURI(icon, ThemeLight)

pkg/octicons/required_icons.txt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Required Octicons for the GitHub MCP Server
2+
# This file is the source of truth for icon requirements.
3+
# Used by:
4+
# - script/fetch-icons (to download icons)
5+
# - pkg/octicons/octicons_test.go (to validate icons are embedded)
6+
# - pkg/github/toolset_icons_test.go (to validate toolset icons exist)
7+
#
8+
# Add new icons here when:
9+
# - Adding a new toolset with an icon
10+
# - Adding a new tool that needs a custom icon
11+
#
12+
# Format: one icon name per line (without -24.svg suffix)
13+
# Lines starting with # are comments
14+
# Empty lines are ignored
15+
16+
apps
17+
beaker
18+
bell
19+
book
20+
check-circle
21+
codescan
22+
comment-discussion
23+
copilot
24+
dependabot
25+
file
26+
git-branch
27+
git-commit
28+
git-merge
29+
git-pull-request
30+
issue-opened
31+
logo-gist
32+
mark-github
33+
organization
34+
people
35+
person
36+
project
37+
repo
38+
repo-forked
39+
shield
40+
shield-lock
41+
star
42+
star-fill
43+
tag
44+
tools
45+
workflow

script/fetch-icons

Lines changed: 16 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,47 +5,17 @@
55
# Requires: rsvg-convert (from librsvg2-bin on Ubuntu/Debian)
66
#
77
# Usage:
8-
# script/fetch-icons # Fetch all default icons
8+
# script/fetch-icons # Fetch all required icons
99
# script/fetch-icons icon1 icon2 # Fetch specific icons
1010

1111
set -e
1212

13-
ICONS_DIR="pkg/octicons/icons"
13+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
14+
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
15+
ICONS_DIR="$REPO_ROOT/pkg/octicons/icons"
16+
REQUIRED_ICONS_FILE="$REPO_ROOT/pkg/octicons/required_icons.txt"
1417
OCTICONS_BASE="https://raw.githubusercontent.com/primer/octicons/main/icons"
1518

16-
# Default icons used by toolsets - add new icons here as needed
17-
DEFAULT_ICONS=(
18-
apps
19-
beaker
20-
bell
21-
check-circle
22-
codescan
23-
comment-discussion
24-
copilot
25-
dependabot
26-
file
27-
git-branch
28-
git-commit
29-
git-merge
30-
git-pull-request
31-
issue-opened
32-
logo-gist
33-
mark-github
34-
organization
35-
people
36-
person
37-
project
38-
repo
39-
repo-forked
40-
shield
41-
shield-lock
42-
star
43-
star-fill
44-
tag
45-
tools
46-
workflow
47-
)
48-
4919
# Check for rsvg-convert
5020
if ! command -v rsvg-convert &> /dev/null; then
5121
echo "Error: rsvg-convert not found. Install with:"
@@ -54,11 +24,16 @@ if ! command -v rsvg-convert &> /dev/null; then
5424
exit 1
5525
fi
5626

57-
# Use provided icons or defaults
27+
# Load icons from required_icons.txt or use command-line arguments
5828
if [ $# -gt 0 ]; then
5929
ICONS=("$@")
6030
else
61-
ICONS=("${DEFAULT_ICONS[@]}")
31+
if [ ! -f "$REQUIRED_ICONS_FILE" ]; then
32+
echo "Error: Required icons file not found: $REQUIRED_ICONS_FILE"
33+
exit 1
34+
fi
35+
# Read icons from file, skipping comments and empty lines
36+
mapfile -t ICONS < <(grep -v '^#' "$REQUIRED_ICONS_FILE" | grep -v '^$')
6237
fi
6338

6439
# Ensure icons directory exists
@@ -91,6 +66,7 @@ done
9166

9267
echo "Done. Icons saved to $ICONS_DIR"
9368
echo ""
94-
echo "Remember to:"
95-
echo " 1. Update pkg/octicons/octicons_test.go if adding new icons"
96-
echo " 2. Run 'UPDATE_TOOLSNAPS=true go test ./...' to update snapshots"
69+
echo "Next steps:"
70+
echo " 1. Run 'go test ./pkg/octicons/...' to verify icons are embedded"
71+
echo " 2. Run 'go test ./pkg/github/...' to verify toolset icons are valid"
72+
echo " 3. Commit the new icon files"

0 commit comments

Comments
 (0)