Skip to content

Commit 8d25f46

Browse files
kerobbiomgitsads
andauthored
Add readonly and toolset request handlers (#1858)
* add readonly and toolset support * address feedback * forgotten files * remove redundant checks in WithRequestConfig * move middleware in RegisterRoutes * improve comment and add TestParseCommaSeparated * fix broken TestInventoryFiltersForRequest * parse X-MCP-Tools into ctx * clean up TestInventoryFiltersForRequest * Pass context to handler, but use request context for per-request data * Pass through the context in MCP server creation functions --------- Co-authored-by: Adam Holt <me@adamholt.co.uk>
1 parent 8d44553 commit 8d25f46

File tree

10 files changed

+326
-41
lines changed

10 files changed

+326
-41
lines changed

internal/ghmcp/server.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv
100100
}, nil
101101
}
102102

103-
func NewStdioMCPServer(cfg github.MCPServerConfig) (*mcp.Server, error) {
103+
func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Server, error) {
104104
apiHost, err := utils.NewAPIHost(cfg.Host)
105105
if err != nil {
106106
return nil, fmt.Errorf("failed to parse API host: %w", err)
@@ -144,7 +144,7 @@ func NewStdioMCPServer(cfg github.MCPServerConfig) (*mcp.Server, error) {
144144
return nil, fmt.Errorf("failed to build inventory: %w", err)
145145
}
146146

147-
ghServer, err := github.NewMCPServer(&cfg, deps, inventory)
147+
ghServer, err := github.NewMCPServer(ctx, &cfg, deps, inventory)
148148
if err != nil {
149149
return nil, fmt.Errorf("failed to create GitHub MCP server: %w", err)
150150
}
@@ -246,7 +246,7 @@ func RunStdioServer(cfg StdioServerConfig) error {
246246
logger.Debug("skipping scope filtering for non-PAT token")
247247
}
248248

249-
ghServer, err := NewStdioMCPServer(github.MCPServerConfig{
249+
ghServer, err := NewStdioMCPServer(ctx, github.MCPServerConfig{
250250
Version: cfg.Version,
251251
Host: cfg.Host,
252252
Token: cfg.Token,

pkg/context/request.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package context
2+
3+
import "context"
4+
5+
// readonlyCtxKey is a context key for read-only mode
6+
type readonlyCtxKey struct{}
7+
8+
// WithReadonly adds read-only mode state to the context
9+
func WithReadonly(ctx context.Context, enabled bool) context.Context {
10+
return context.WithValue(ctx, readonlyCtxKey{}, enabled)
11+
}
12+
13+
// IsReadonly retrieves the read-only mode state from the context
14+
func IsReadonly(ctx context.Context) bool {
15+
if enabled, ok := ctx.Value(readonlyCtxKey{}).(bool); ok {
16+
return enabled
17+
}
18+
return false
19+
}
20+
21+
// toolsetsCtxKey is a context key for the active toolsets
22+
type toolsetsCtxKey struct{}
23+
24+
// WithToolsets adds the active toolsets to the context
25+
func WithToolsets(ctx context.Context, toolsets []string) context.Context {
26+
return context.WithValue(ctx, toolsetsCtxKey{}, toolsets)
27+
}
28+
29+
// GetToolsets retrieves the active toolsets from the context
30+
func GetToolsets(ctx context.Context) []string {
31+
if toolsets, ok := ctx.Value(toolsetsCtxKey{}).([]string); ok {
32+
return toolsets
33+
}
34+
return nil
35+
}
36+
37+
// toolsCtxKey is a context key for tools
38+
type toolsCtxKey struct{}
39+
40+
// WithTools adds the tools to the context
41+
func WithTools(ctx context.Context, tools []string) context.Context {
42+
return context.WithValue(ctx, toolsCtxKey{}, tools)
43+
}
44+
45+
// GetTools retrieves the tools from the context
46+
func GetTools(ctx context.Context) []string {
47+
if tools, ok := ctx.Value(toolsCtxKey{}).([]string); ok {
48+
return tools
49+
}
50+
return nil
51+
}

pkg/github/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ type MCPServerConfig struct {
7373

7474
type MCPServerOption func(*mcp.ServerOptions)
7575

76-
func NewMCPServer(cfg *MCPServerConfig, deps ToolDependencies, inventory *inventory.Inventory) (*mcp.Server, error) {
76+
func NewMCPServer(ctx context.Context, cfg *MCPServerConfig, deps ToolDependencies, inventory *inventory.Inventory) (*mcp.Server, error) {
7777
// Create the MCP server
7878
serverOpts := &mcp.ServerOptions{
7979
Instructions: inventory.Instructions(),
@@ -110,7 +110,7 @@ func NewMCPServer(cfg *MCPServerConfig, deps ToolDependencies, inventory *invent
110110
// In dynamic mode with no explicit toolsets, this is a no-op since enabledToolsets
111111
// is empty - users enable toolsets at runtime via the dynamic tools below (but can
112112
// enable toolsets or tools explicitly that do need registration).
113-
inventory.RegisterAll(context.Background(), ghServer, deps)
113+
inventory.RegisterAll(ctx, ghServer, deps)
114114

115115
// Register dynamic toolset management tools (enable/disable) - these are separate
116116
// meta-tools that control the inventory, not part of the inventory itself

pkg/github/server_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func TestNewMCPServer_CreatesSuccessfully(t *testing.T) {
134134
require.NoError(t, err, "expected inventory build to succeed")
135135

136136
// Create the server
137-
server, err := NewMCPServer(&cfg, deps, inv)
137+
server, err := NewMCPServer(t.Context(), &cfg, deps, inv)
138138
require.NoError(t, err, "expected server creation to succeed")
139139
require.NotNil(t, server, "expected server to be non-nil")
140140

pkg/http/handler.go

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import (
44
"context"
55
"log/slog"
66
"net/http"
7-
"strings"
87

8+
ghcontext "github.com/github/github-mcp-server/pkg/context"
99
"github.com/github/github-mcp-server/pkg/github"
1010
"github.com/github/github-mcp-server/pkg/http/headers"
1111
"github.com/github/github-mcp-server/pkg/http/middleware"
@@ -16,9 +16,10 @@ import (
1616
)
1717

1818
type InventoryFactoryFunc func(r *http.Request) (*inventory.Inventory, error)
19-
type GitHubMCPServerFactoryFunc func(ctx context.Context, r *http.Request, deps github.ToolDependencies, inventory *inventory.Inventory, cfg *github.MCPServerConfig) (*mcp.Server, error)
19+
type GitHubMCPServerFactoryFunc func(r *http.Request, deps github.ToolDependencies, inventory *inventory.Inventory, cfg *github.MCPServerConfig) (*mcp.Server, error)
2020

2121
type HTTPMcpHandler struct {
22+
ctx context.Context
2223
config *HTTPServerConfig
2324
deps github.ToolDependencies
2425
logger *slog.Logger
@@ -46,7 +47,9 @@ func WithInventoryFactory(f InventoryFactoryFunc) HTTPMcpHandlerOption {
4647
}
4748
}
4849

49-
func NewHTTPMcpHandler(cfg *HTTPServerConfig,
50+
func NewHTTPMcpHandler(
51+
ctx context.Context,
52+
cfg *HTTPServerConfig,
5053
deps github.ToolDependencies,
5154
t translations.TranslationHelperFunc,
5255
logger *slog.Logger,
@@ -67,6 +70,7 @@ func NewHTTPMcpHandler(cfg *HTTPServerConfig,
6770
}
6871

6972
return &HTTPMcpHandler{
73+
ctx: ctx,
7074
config: cfg,
7175
deps: deps,
7276
logger: logger,
@@ -76,8 +80,33 @@ func NewHTTPMcpHandler(cfg *HTTPServerConfig,
7680
}
7781
}
7882

83+
// RegisterRoutes registers the routes for the MCP server
84+
// URL-based values take precedence over header-based values
7985
func (h *HTTPMcpHandler) RegisterRoutes(r chi.Router) {
86+
r.Use(middleware.WithRequestConfig)
87+
8088
r.Mount("/", h)
89+
// Mount readonly and toolset routes
90+
r.With(withToolset).Mount("/x/{toolset}", h)
91+
r.With(withReadonly, withToolset).Mount("/x/{toolset}/readonly", h)
92+
r.With(withReadonly).Mount("/readonly", h)
93+
}
94+
95+
// withReadonly is middleware that sets readonly mode in the request context
96+
func withReadonly(next http.Handler) http.Handler {
97+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
98+
ctx := ghcontext.WithReadonly(r.Context(), true)
99+
next.ServeHTTP(w, r.WithContext(ctx))
100+
})
101+
}
102+
103+
// withToolset is middleware that extracts the toolset from the URL and sets it in the request context
104+
func withToolset(next http.Handler) http.Handler {
105+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
106+
toolset := chi.URLParam(r, "toolset")
107+
ctx := ghcontext.WithToolsets(r.Context(), []string{toolset})
108+
next.ServeHTTP(w, r.WithContext(ctx))
109+
})
81110
}
82111

83112
func (h *HTTPMcpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -87,7 +116,7 @@ func (h *HTTPMcpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
87116
return
88117
}
89118

90-
ghServer, err := h.githubMcpServerFactory(r.Context(), r, h.deps, inventory, &github.MCPServerConfig{
119+
ghServer, err := h.githubMcpServerFactory(r, h.deps, inventory, &github.MCPServerConfig{
91120
Version: h.config.Version,
92121
Translator: h.t,
93122
ContentWindowSize: h.config.ContentWindowSize,
@@ -108,8 +137,8 @@ func (h *HTTPMcpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
108137
middleware.ExtractUserToken()(mcpHandler).ServeHTTP(w, r)
109138
}
110139

111-
func DefaultGitHubMCPServerFactory(ctx context.Context, _ *http.Request, deps github.ToolDependencies, inventory *inventory.Inventory, cfg *github.MCPServerConfig) (*mcp.Server, error) {
112-
return github.NewMCPServer(&github.MCPServerConfig{
140+
func DefaultGitHubMCPServerFactory(r *http.Request, deps github.ToolDependencies, inventory *inventory.Inventory, cfg *github.MCPServerConfig) (*mcp.Server, error) {
141+
return github.NewMCPServer(r.Context(), &github.MCPServerConfig{
113142
Version: cfg.Version,
114143
Translator: cfg.Translator,
115144
ContentWindowSize: cfg.ContentWindowSize,
@@ -123,52 +152,37 @@ func DefaultInventoryFactory(cfg *HTTPServerConfig, t translations.TranslationHe
123152
b := github.NewInventory(t).WithDeprecatedAliases(github.DeprecatedToolAliases)
124153

125154
// Feature checker composition
126-
headerFeatures := parseCommaSeparatedHeader(r.Header.Get(headers.MCPFeaturesHeader))
155+
headerFeatures := headers.ParseCommaSeparated(r.Header.Get(headers.MCPFeaturesHeader))
127156
if checker := ComposeFeatureChecker(headerFeatures, staticChecker); checker != nil {
128157
b = b.WithFeatureChecker(checker)
129158
}
130159

131-
b = InventoryFiltersForRequestHeaders(r, b)
160+
b = InventoryFiltersForRequest(r, b)
132161
b.WithServerInstructions()
133162

134163
return b.Build()
135164
}
136165
}
137166

138-
// InventoryFiltersForRequestHeaders applies inventory filters based on HTTP request headers.
139-
// Whitespace is trimmed from comma-separated values; empty values are ignored.
140-
func InventoryFiltersForRequestHeaders(r *http.Request, builder *inventory.Builder) *inventory.Builder {
141-
if r.Header.Get(headers.MCPReadOnlyHeader) != "" {
167+
// InventoryFiltersForRequest applies filters to the inventory builder
168+
// based on the request context and headers
169+
func InventoryFiltersForRequest(r *http.Request, builder *inventory.Builder) *inventory.Builder {
170+
ctx := r.Context()
171+
172+
if ghcontext.IsReadonly(ctx) {
142173
builder = builder.WithReadOnly(true)
143174
}
144175

145-
if toolsetsStr := r.Header.Get(headers.MCPToolsetsHeader); toolsetsStr != "" {
146-
toolsets := parseCommaSeparatedHeader(toolsetsStr)
176+
if toolsets := ghcontext.GetToolsets(ctx); len(toolsets) > 0 {
147177
builder = builder.WithToolsets(toolsets)
148178
}
149179

150-
if toolsStr := r.Header.Get(headers.MCPToolsHeader); toolsStr != "" {
151-
tools := parseCommaSeparatedHeader(toolsStr)
180+
if tools := ghcontext.GetTools(ctx); len(tools) > 0 {
181+
if len(ghcontext.GetToolsets(ctx)) == 0 {
182+
builder = builder.WithToolsets([]string{})
183+
}
152184
builder = builder.WithTools(github.CleanTools(tools))
153185
}
154186

155187
return builder
156188
}
157-
158-
// parseCommaSeparatedHeader splits a header value by comma, trims whitespace,
159-
// and filters out empty values.
160-
func parseCommaSeparatedHeader(value string) []string {
161-
if value == "" {
162-
return []string{}
163-
}
164-
165-
parts := strings.Split(value, ",")
166-
result := make([]string, 0, len(parts))
167-
for _, p := range parts {
168-
trimmed := strings.TrimSpace(p)
169-
if trimmed != "" {
170-
result = append(result, trimmed)
171-
}
172-
}
173-
return result
174-
}

pkg/http/handler_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package http
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
ghcontext "github.com/github/github-mcp-server/pkg/context"
10+
"github.com/github/github-mcp-server/pkg/inventory"
11+
"github.com/modelcontextprotocol/go-sdk/mcp"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
func mockTool(name, toolsetID string, readOnly bool) inventory.ServerTool {
17+
return inventory.ServerTool{
18+
Tool: mcp.Tool{
19+
Name: name,
20+
Annotations: &mcp.ToolAnnotations{ReadOnlyHint: readOnly},
21+
},
22+
Toolset: inventory.ToolsetMetadata{
23+
ID: inventory.ToolsetID(toolsetID),
24+
Description: "Test: " + toolsetID,
25+
},
26+
}
27+
}
28+
29+
func TestInventoryFiltersForRequest(t *testing.T) {
30+
tools := []inventory.ServerTool{
31+
mockTool("get_file_contents", "repos", true),
32+
mockTool("create_repository", "repos", false),
33+
mockTool("list_issues", "issues", true),
34+
mockTool("issue_write", "issues", false),
35+
}
36+
37+
tests := []struct {
38+
name string
39+
contextSetup func(context.Context) context.Context
40+
expectedTools []string
41+
}{
42+
{
43+
name: "no filters applies defaults",
44+
contextSetup: func(ctx context.Context) context.Context { return ctx },
45+
expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "issue_write"},
46+
},
47+
{
48+
name: "readonly from context filters write tools",
49+
contextSetup: func(ctx context.Context) context.Context {
50+
return ghcontext.WithReadonly(ctx, true)
51+
},
52+
expectedTools: []string{"get_file_contents", "list_issues"},
53+
},
54+
{
55+
name: "toolset from context filters to toolset",
56+
contextSetup: func(ctx context.Context) context.Context {
57+
return ghcontext.WithToolsets(ctx, []string{"repos"})
58+
},
59+
expectedTools: []string{"get_file_contents", "create_repository"},
60+
},
61+
{
62+
name: "tools alone clears default toolsets",
63+
contextSetup: func(ctx context.Context) context.Context {
64+
return ghcontext.WithTools(ctx, []string{"list_issues"})
65+
},
66+
expectedTools: []string{"list_issues"},
67+
},
68+
{
69+
name: "tools are additive with toolsets",
70+
contextSetup: func(ctx context.Context) context.Context {
71+
ctx = ghcontext.WithToolsets(ctx, []string{"repos"})
72+
ctx = ghcontext.WithTools(ctx, []string{"list_issues"})
73+
return ctx
74+
},
75+
expectedTools: []string{"get_file_contents", "create_repository", "list_issues"},
76+
},
77+
}
78+
79+
for _, tt := range tests {
80+
t.Run(tt.name, func(t *testing.T) {
81+
req := httptest.NewRequest(http.MethodGet, "/", nil)
82+
req = req.WithContext(tt.contextSetup(req.Context()))
83+
84+
builder := inventory.NewBuilder().
85+
SetTools(tools).
86+
WithToolsets([]string{"all"})
87+
88+
builder = InventoryFiltersForRequest(req, builder)
89+
inv, err := builder.Build()
90+
require.NoError(t, err)
91+
92+
available := inv.AvailableTools(context.Background())
93+
toolNames := make([]string, len(available))
94+
for i, tool := range available {
95+
toolNames[i] = tool.Tool.Name
96+
}
97+
98+
assert.ElementsMatch(t, tt.expectedTools, toolNames)
99+
})
100+
}
101+
}

pkg/http/headers/parse.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package headers
2+
3+
import "strings"
4+
5+
// ParseCommaSeparated splits a header value by comma, trims whitespace,
6+
// and filters out empty values
7+
func ParseCommaSeparated(value string) []string {
8+
if value == "" {
9+
return []string{}
10+
}
11+
12+
parts := strings.Split(value, ",")
13+
result := make([]string, 0, len(parts))
14+
for _, p := range parts {
15+
trimmed := strings.TrimSpace(p)
16+
if trimmed != "" {
17+
result = append(result, trimmed)
18+
}
19+
}
20+
return result
21+
}

0 commit comments

Comments
 (0)