|
| 1 | +package github |
| 2 | + |
| 3 | +import ( |
| 4 | + "context" |
| 5 | + "encoding/json" |
| 6 | + "strings" |
| 7 | + "testing" |
| 8 | + |
| 9 | + "github.com/github/github-mcp-server/pkg/translations" |
| 10 | + "github.com/github/github-mcp-server/skills" |
| 11 | + "github.com/modelcontextprotocol/go-sdk/mcp" |
| 12 | + "github.com/stretchr/testify/assert" |
| 13 | + "github.com/stretchr/testify/require" |
| 14 | +) |
| 15 | + |
| 16 | +// pullRequestsSkillURI is the canonical URI of the bundled pull-requests |
| 17 | +// skill, derived from the skills.Bundled entry so tests never drift from |
| 18 | +// the single source of truth. |
| 19 | +var pullRequestsSkillURI = skills.Bundled{Name: "pull-requests"}.URI() |
| 20 | + |
| 21 | +// Test_PullRequestsSkill_EmbeddedContent verifies the SEP structural requirement |
| 22 | +// that the frontmatter `name` field matches the final segment of the skill-path |
| 23 | +// in the URI, and that the substantive tool-sequence content is preserved. |
| 24 | +func Test_PullRequestsSkill_EmbeddedContent(t *testing.T) { |
| 25 | + require.NotEmpty(t, skills.PullRequestsSKILL, "SKILL.md must be embedded") |
| 26 | + |
| 27 | + // Normalize line endings so the test is robust to git's autocrlf behavior |
| 28 | + // on Windows checkouts — the embedded SKILL.md may arrive as CRLF. |
| 29 | + md := strings.ReplaceAll(skills.PullRequestsSKILL, "\r\n", "\n") |
| 30 | + require.True(t, strings.HasPrefix(md, "---\n"), "SKILL.md must begin with YAML frontmatter") |
| 31 | + |
| 32 | + end := strings.Index(md[4:], "\n---\n") |
| 33 | + require.GreaterOrEqual(t, end, 0, "SKILL.md must have closing frontmatter fence") |
| 34 | + frontmatter := md[4 : 4+end] |
| 35 | + |
| 36 | + var frontmatterName string |
| 37 | + for _, line := range strings.Split(frontmatter, "\n") { |
| 38 | + if strings.HasPrefix(line, "name:") { |
| 39 | + frontmatterName = strings.TrimSpace(strings.TrimPrefix(line, "name:")) |
| 40 | + break |
| 41 | + } |
| 42 | + } |
| 43 | + require.NotEmpty(t, frontmatterName, "SKILL.md frontmatter must declare `name`") |
| 44 | + assert.Equal(t, "pull-requests", frontmatterName, "frontmatter name must match final skill-path segment in %s", pullRequestsSkillURI) |
| 45 | + |
| 46 | + body := md[4+end+5:] |
| 47 | + assert.Contains(t, body, "## PR review workflow") |
| 48 | + assert.Contains(t, body, "pull_request_review_write", "review workflow content must be preserved") |
| 49 | + assert.Contains(t, body, "submit_pending", "the distinctive tool method must be present") |
| 50 | +} |
| 51 | + |
| 52 | +// Test_BundledSkills_Registration verifies that skill resources are |
| 53 | +// registered when the backing toolset is enabled, and omitted when it is not. |
| 54 | +func Test_BundledSkills_Registration(t *testing.T) { |
| 55 | + ctx := context.Background() |
| 56 | + |
| 57 | + t.Run("registers when pull_requests toolset enabled", func(t *testing.T) { |
| 58 | + inv, err := NewInventory(translations.NullTranslationHelper). |
| 59 | + WithToolsets([]string{string(ToolsetMetadataPullRequests.ID)}). |
| 60 | + Build() |
| 61 | + require.NoError(t, err) |
| 62 | + |
| 63 | + srv := mcp.NewServer(&mcp.Implementation{Name: "test"}, &mcp.ServerOptions{ |
| 64 | + Capabilities: &mcp.ServerCapabilities{Resources: &mcp.ResourceCapabilities{}}, |
| 65 | + }) |
| 66 | + RegisterBundledSkills(srv, inv) |
| 67 | + |
| 68 | + mimes := map[string]string{} |
| 69 | + for _, r := range listResources(t, ctx, srv) { |
| 70 | + mimes[r.URI] = r.MIMEType |
| 71 | + } |
| 72 | + assert.Equal(t, "text/markdown", mimes[pullRequestsSkillURI]) |
| 73 | + assert.Equal(t, "application/json", mimes[skills.IndexURI]) |
| 74 | + }) |
| 75 | + |
| 76 | + t.Run("omits when pull_requests toolset disabled", func(t *testing.T) { |
| 77 | + inv, err := NewInventory(translations.NullTranslationHelper). |
| 78 | + WithToolsets([]string{string(ToolsetMetadataContext.ID)}). |
| 79 | + Build() |
| 80 | + require.NoError(t, err) |
| 81 | + |
| 82 | + srv := mcp.NewServer(&mcp.Implementation{Name: "test"}, &mcp.ServerOptions{ |
| 83 | + Capabilities: &mcp.ServerCapabilities{Resources: &mcp.ResourceCapabilities{}}, |
| 84 | + }) |
| 85 | + RegisterBundledSkills(srv, inv) |
| 86 | + |
| 87 | + for _, r := range listResources(t, ctx, srv) { |
| 88 | + assert.NotEqual(t, pullRequestsSkillURI, r.URI) |
| 89 | + assert.NotEqual(t, skills.IndexURI, r.URI) |
| 90 | + } |
| 91 | + }) |
| 92 | +} |
| 93 | + |
| 94 | +// Test_BundledSkills_ReadContent verifies that reading the skill resource |
| 95 | +// returns the embedded SKILL.md content, and the index resource returns a JSON |
| 96 | +// document matching the SEP discovery schema shape. |
| 97 | +func Test_BundledSkills_ReadContent(t *testing.T) { |
| 98 | + ctx := context.Background() |
| 99 | + inv, err := NewInventory(translations.NullTranslationHelper). |
| 100 | + WithToolsets([]string{string(ToolsetMetadataPullRequests.ID)}). |
| 101 | + Build() |
| 102 | + require.NoError(t, err) |
| 103 | + |
| 104 | + srv := mcp.NewServer(&mcp.Implementation{Name: "test"}, &mcp.ServerOptions{ |
| 105 | + Capabilities: &mcp.ServerCapabilities{Resources: &mcp.ResourceCapabilities{}}, |
| 106 | + }) |
| 107 | + RegisterBundledSkills(srv, inv) |
| 108 | + |
| 109 | + session := connectClient(t, ctx, srv) |
| 110 | + |
| 111 | + t.Run("SKILL.md content", func(t *testing.T) { |
| 112 | + res, err := session.ReadResource(ctx, &mcp.ReadResourceParams{URI: pullRequestsSkillURI}) |
| 113 | + require.NoError(t, err) |
| 114 | + require.Len(t, res.Contents, 1) |
| 115 | + assert.Equal(t, "text/markdown", res.Contents[0].MIMEType) |
| 116 | + assert.Equal(t, skills.PullRequestsSKILL, res.Contents[0].Text) |
| 117 | + }) |
| 118 | + |
| 119 | + t.Run("index.json matches SEP discovery schema", func(t *testing.T) { |
| 120 | + res, err := session.ReadResource(ctx, &mcp.ReadResourceParams{URI: skills.IndexURI}) |
| 121 | + require.NoError(t, err) |
| 122 | + require.Len(t, res.Contents, 1) |
| 123 | + assert.Equal(t, "application/json", res.Contents[0].MIMEType) |
| 124 | + |
| 125 | + var idx skills.IndexDoc |
| 126 | + require.NoError(t, json.Unmarshal([]byte(res.Contents[0].Text), &idx)) |
| 127 | + assert.Equal(t, skills.IndexSchema, idx.Schema) |
| 128 | + require.Len(t, idx.Skills, 1) |
| 129 | + assert.Equal(t, "pull-requests", idx.Skills[0].Name) |
| 130 | + assert.Equal(t, "skill-md", idx.Skills[0].Type) |
| 131 | + assert.Equal(t, pullRequestsSkillURI, idx.Skills[0].URL) |
| 132 | + assert.NotEmpty(t, idx.Skills[0].Description) |
| 133 | + }) |
| 134 | +} |
| 135 | + |
| 136 | +// Test_DeclareSkillsExtensionIfEnabled verifies that the skills-over-MCP |
| 137 | +// extension (SEP-2133) is declared in ServerOptions.Capabilities when the |
| 138 | +// pull_requests toolset is enabled, and is absent when it is not. |
| 139 | +func Test_DeclareSkillsExtensionIfEnabled(t *testing.T) { |
| 140 | + t.Run("declares when pull_requests enabled", func(t *testing.T) { |
| 141 | + inv, err := NewInventory(translations.NullTranslationHelper). |
| 142 | + WithToolsets([]string{string(ToolsetMetadataPullRequests.ID)}). |
| 143 | + Build() |
| 144 | + require.NoError(t, err) |
| 145 | + |
| 146 | + opts := &mcp.ServerOptions{} |
| 147 | + DeclareSkillsExtensionIfEnabled(opts, inv) |
| 148 | + |
| 149 | + require.NotNil(t, opts.Capabilities) |
| 150 | + _, ok := opts.Capabilities.Extensions[skills.ExtensionKey] |
| 151 | + assert.True(t, ok, "skills extension must be declared") |
| 152 | + }) |
| 153 | + |
| 154 | + t.Run("does not declare when pull_requests disabled", func(t *testing.T) { |
| 155 | + inv, err := NewInventory(translations.NullTranslationHelper). |
| 156 | + WithToolsets([]string{string(ToolsetMetadataContext.ID)}). |
| 157 | + Build() |
| 158 | + require.NoError(t, err) |
| 159 | + |
| 160 | + opts := &mcp.ServerOptions{} |
| 161 | + DeclareSkillsExtensionIfEnabled(opts, inv) |
| 162 | + |
| 163 | + if opts.Capabilities != nil { |
| 164 | + _, ok := opts.Capabilities.Extensions[skills.ExtensionKey] |
| 165 | + assert.False(t, ok, "skills extension must NOT be declared when no skills will be registered") |
| 166 | + } |
| 167 | + }) |
| 168 | + |
| 169 | + t.Run("preserves other extensions already declared", func(t *testing.T) { |
| 170 | + inv, err := NewInventory(translations.NullTranslationHelper). |
| 171 | + WithToolsets([]string{string(ToolsetMetadataPullRequests.ID)}). |
| 172 | + Build() |
| 173 | + require.NoError(t, err) |
| 174 | + |
| 175 | + opts := &mcp.ServerOptions{ |
| 176 | + Capabilities: &mcp.ServerCapabilities{}, |
| 177 | + } |
| 178 | + opts.Capabilities.AddExtension("io.example/other", map[string]any{"k": "v"}) |
| 179 | + |
| 180 | + DeclareSkillsExtensionIfEnabled(opts, inv) |
| 181 | + |
| 182 | + _, hasSkills := opts.Capabilities.Extensions[skills.ExtensionKey] |
| 183 | + _, hasOther := opts.Capabilities.Extensions["io.example/other"] |
| 184 | + assert.True(t, hasSkills) |
| 185 | + assert.True(t, hasOther, "existing extensions must not be overwritten") |
| 186 | + }) |
| 187 | +} |
| 188 | + |
| 189 | +// listResources enumerates resources/list via an in-memory client session. |
| 190 | +func listResources(t *testing.T, ctx context.Context, srv *mcp.Server) []*mcp.Resource { |
| 191 | + t.Helper() |
| 192 | + session := connectClient(t, ctx, srv) |
| 193 | + res, err := session.ListResources(ctx, &mcp.ListResourcesParams{}) |
| 194 | + require.NoError(t, err) |
| 195 | + return res.Resources |
| 196 | +} |
| 197 | + |
| 198 | +// connectClient wires an in-memory transport and returns a connected client session. |
| 199 | +func connectClient(t *testing.T, ctx context.Context, srv *mcp.Server) *mcp.ClientSession { |
| 200 | + t.Helper() |
| 201 | + clientT, serverT := mcp.NewInMemoryTransports() |
| 202 | + _, err := srv.Connect(ctx, serverT, nil) |
| 203 | + require.NoError(t, err) |
| 204 | + |
| 205 | + client := mcp.NewClient(&mcp.Implementation{Name: "test-client"}, nil) |
| 206 | + session, err := client.Connect(ctx, clientT, nil) |
| 207 | + require.NoError(t, err) |
| 208 | + t.Cleanup(func() { _ = session.Close() }) |
| 209 | + return session |
| 210 | +} |
0 commit comments