Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit311d581

Browse files
Fix: Add middleware to inject deps into context for tool handlers
The issue was that after PR#1640 switched from closure-based deps to context-based deps,the stdio server was missing middleware to inject ToolDependencies into the request context.This caused tools to panic with "ToolDependencies not found in context" when called.Added middleware in NewMCPServer() that wraps all requests with github.ContextWithDeps(),ensuring deps are available to tool handlers via MustDepsFromContext().Also added tests to verify server creation and toolset resolution logic.Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parenta8fafad commit311d581

File tree

2 files changed

+119
-0
lines changed

2 files changed

+119
-0
lines changed

‎internal/ghmcp/server.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,13 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
203203
cfg.ContentWindowSize,
204204
)
205205

206+
// Inject dependencies into context for all tool handlers
207+
ghServer.AddReceivingMiddleware(func(next mcp.MethodHandler) mcp.MethodHandler {
208+
returnfunc(ctx context.Context,methodstring,req mcp.Request) (mcp.Result,error) {
209+
returnnext(github.ContextWithDeps(ctx,deps),method,req)
210+
}
211+
})
212+
206213
// Build and register the tool/resource/prompt inventory
207214
inventory:=github.NewInventory(cfg.Translator).
208215
WithDeprecatedAliases(github.DeprecatedToolAliases).

‎internal/ghmcp/server_test.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package ghmcp
2+
3+
import (
4+
"testing"
5+
6+
"github.com/github/github-mcp-server/pkg/translations"
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// TestNewMCPServer_CreatesSuccessfully verifies that the server can be created
12+
// with the deps injection middleware properly configured.
13+
funcTestNewMCPServer_CreatesSuccessfully(t*testing.T) {
14+
t.Parallel()
15+
16+
// Create a minimal server configuration
17+
cfg:=MCPServerConfig{
18+
Version:"test",
19+
Host:"",// defaults to github.com
20+
Token:"test-token",
21+
EnabledToolsets: []string{"context"},
22+
ReadOnly:false,
23+
Translator:translations.NullTranslationHelper,
24+
ContentWindowSize:5000,
25+
LockdownMode:false,
26+
}
27+
28+
// Create the server
29+
server,err:=NewMCPServer(cfg)
30+
require.NoError(t,err,"expected server creation to succeed")
31+
require.NotNil(t,server,"expected server to be non-nil")
32+
33+
// The fact that the server was created successfully indicates that:
34+
// 1. The deps injection middleware is properly added
35+
// 2. Tools can be registered without panicking
36+
//
37+
// If the middleware wasn't properly added, tool calls would panic with
38+
// "ToolDependencies not found in context" when executed.
39+
//
40+
// The actual middleware functionality and tool execution with ContextWithDeps
41+
// is already tested in pkg/github/*_test.go.
42+
}
43+
44+
// TestResolveEnabledToolsets verifies the toolset resolution logic.
45+
funcTestResolveEnabledToolsets(t*testing.T) {
46+
t.Parallel()
47+
48+
tests:= []struct {
49+
namestring
50+
cfgMCPServerConfig
51+
expectedResult []string
52+
}{
53+
{
54+
name:"nil toolsets without dynamic mode and no tools - use defaults",
55+
cfg:MCPServerConfig{
56+
EnabledToolsets:nil,
57+
DynamicToolsets:false,
58+
EnabledTools:nil,
59+
},
60+
expectedResult:nil,// nil means "use defaults"
61+
},
62+
{
63+
name:"nil toolsets with dynamic mode - start empty",
64+
cfg:MCPServerConfig{
65+
EnabledToolsets:nil,
66+
DynamicToolsets:true,
67+
EnabledTools:nil,
68+
},
69+
expectedResult: []string{},// empty slice means no toolsets
70+
},
71+
{
72+
name:"explicit toolsets",
73+
cfg:MCPServerConfig{
74+
EnabledToolsets: []string{"repos","issues"},
75+
DynamicToolsets:false,
76+
},
77+
expectedResult: []string{"repos","issues"},
78+
},
79+
{
80+
name:"empty toolsets - disable all",
81+
cfg:MCPServerConfig{
82+
EnabledToolsets: []string{},
83+
DynamicToolsets:false,
84+
},
85+
expectedResult: []string{},// empty slice means no toolsets
86+
},
87+
{
88+
name:"specific tools without toolsets - no default toolsets",
89+
cfg:MCPServerConfig{
90+
EnabledToolsets:nil,
91+
DynamicToolsets:false,
92+
EnabledTools: []string{"get_me"},
93+
},
94+
expectedResult: []string{},// empty slice when tools specified but no toolsets
95+
},
96+
{
97+
name:"dynamic mode with explicit toolsets removes all and default",
98+
cfg:MCPServerConfig{
99+
EnabledToolsets: []string{"all","repos"},
100+
DynamicToolsets:true,
101+
},
102+
expectedResult: []string{"repos"},// "all" is removed in dynamic mode
103+
},
104+
}
105+
106+
for_,tc:=rangetests {
107+
t.Run(tc.name,func(t*testing.T) {
108+
result:=resolveEnabledToolsets(tc.cfg)
109+
assert.Equal(t,tc.expectedResult,result)
110+
})
111+
}
112+
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp