Skip to content

Commit c5db3b2

Browse files
authored
feat: add description, allowedTools, and allowedResources options (#17)
* feat: add description, allowedTools, and allowedResources options * test: consolidate startFakeServer helpers and add E2E options tests
1 parent 4e21e70 commit c5db3b2

9 files changed

Lines changed: 402 additions & 44 deletions

File tree

README.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,19 @@ Create an `mcp.json` file:
7878
Each entry in `mcpServers` is a downstream server that becomes a skill. The key
7979
is the skill name. The value depends on the transport type:
8080

81+
### Common options
82+
83+
All server types support these optional fields:
84+
85+
| Field | Description |
86+
|-------|-------------|
87+
| `description` | Override the server's instructions shown by `list_skills` |
88+
| `allowedTools` | Only expose these tool names (default: all) |
89+
| `allowedResources` | Only expose these resource URIs (default: all) |
90+
91+
Excluded tools are invisible everywhere — they won't appear in `use_skill`,
92+
can't be called via `execute_code`, and won't cause name-conflict prefixing.
93+
8194
### STDIO server
8295

8396
Spawns the server as a child process. Only env vars explicitly listed in `env`
@@ -95,6 +108,8 @@ are passed to the child — the parent environment is not inherited.
95108
"database": {
96109
"command": "npx",
97110
"args": ["-y", "@modelcontextprotocol/server-sqlite", "mydb.db"],
111+
"description": "SQL database for customer analytics",
112+
"allowedTools": ["execute_sql", "list_tables"],
98113
"env": {
99114
"PATH": "/usr/local/bin:/usr/bin"
100115
}
@@ -159,6 +174,7 @@ go run . --config mcp.json --transport http --port 8080
159174
| `--transport` | `stdio` | Upstream transport: `stdio` or `http` |
160175
| `--host` | `localhost` | HTTP listen host |
161176
| `--port` | `8080` | HTTP listen port |
177+
| `--version` | | Print version and exit |
162178

163179
### Use with MCP clients
164180

internal/config/config.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const (
1919
// Server is the common interface for all server transport configurations.
2020
type Server interface {
2121
TransportType() TransportType
22+
Options() ServerOptions
2223
}
2324

2425
func Load(path string) (map[string]Server, error) {
@@ -97,13 +98,23 @@ func unmarshalStrict(data []byte, v any) error {
9798
return dec.Decode(v)
9899
}
99100

101+
// ServerOptions contains optional fields shared by all server types.
102+
type ServerOptions struct {
103+
Description string `json:"description,omitempty"`
104+
AllowedTools []string `json:"allowedTools,omitempty"`
105+
AllowedResources []string `json:"allowedResources,omitempty"`
106+
}
107+
108+
func (o ServerOptions) Options() ServerOptions { return o }
109+
100110
// Server types
101111

102112
type StdioServer struct {
103113
Type string `json:"type"`
104114
Command string `json:"command"`
105115
Args []string `json:"args,omitempty"`
106116
Env map[string]string `json:"env,omitempty"`
117+
ServerOptions
107118
}
108119

109120
func (*StdioServer) TransportType() TransportType { return TransportSTDIO }
@@ -112,6 +123,7 @@ type HTTPServer struct {
112123
Type string `json:"type"`
113124
URL string `json:"url"`
114125
Headers map[string]string `json:"headers,omitempty"`
126+
ServerOptions
115127
}
116128

117129
func (*HTTPServer) TransportType() TransportType { return TransportHTTP }
@@ -120,6 +132,7 @@ type SSEServer struct {
120132
Type string `json:"type"`
121133
URL string `json:"url"`
122134
Headers map[string]string `json:"headers,omitempty"`
135+
ServerOptions
123136
}
124137

125138
func (*SSEServer) TransportType() TransportType { return TransportSSE }

internal/config/config_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,54 @@ func TestLoadedFieldValues(t *testing.T) {
199199
}
200200
}
201201

202+
func TestServerOptions(t *testing.T) {
203+
t.Parallel()
204+
205+
jsonStr := `{
206+
"mcpServers": {
207+
"db": {
208+
"command": "db-server",
209+
"description": "SQL database for analytics",
210+
"allowedTools": ["execute_sql", "list_tables"],
211+
"allowedResources": ["schema://tables"]
212+
},
213+
"plain": {
214+
"command": "plain-server"
215+
}
216+
}
217+
}`
218+
path := writeTestConfig(t, jsonStr)
219+
servers, err := Load(path)
220+
if err != nil {
221+
t.Fatal(err)
222+
}
223+
224+
db := servers["db"].(*StdioServer)
225+
opts := db.Options()
226+
if opts.Description != "SQL database for analytics" {
227+
t.Errorf("Description = %q, want 'SQL database for analytics'", opts.Description)
228+
}
229+
if len(opts.AllowedTools) != 2 || opts.AllowedTools[0] != "execute_sql" || opts.AllowedTools[1] != "list_tables" {
230+
t.Errorf("AllowedTools = %v, want [execute_sql list_tables]", opts.AllowedTools)
231+
}
232+
if len(opts.AllowedResources) != 1 || opts.AllowedResources[0] != "schema://tables" {
233+
t.Errorf("AllowedResources = %v, want [schema://tables]", opts.AllowedResources)
234+
}
235+
236+
// Server with no options should have zero values.
237+
plain := servers["plain"].(*StdioServer)
238+
plainOpts := plain.Options()
239+
if plainOpts.Description != "" {
240+
t.Errorf("expected empty description, got %q", plainOpts.Description)
241+
}
242+
if len(plainOpts.AllowedTools) != 0 {
243+
t.Errorf("expected no allowed tools, got %v", plainOpts.AllowedTools)
244+
}
245+
if len(plainOpts.AllowedResources) != 0 {
246+
t.Errorf("expected no allowed resources, got %v", plainOpts.AllowedResources)
247+
}
248+
}
249+
202250
func TestEnvVarExpansion(t *testing.T) {
203251
t.Setenv("TEST_CMD", "my-server")
204252
t.Setenv("TEST_ARG", "--verbose")

internal/e2e/e2e_test.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strings"
77
"testing"
88

9+
"skillful-mcp/internal/config"
910
"skillful-mcp/internal/mcpserver"
1011

1112
"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -521,3 +522,134 @@ func TestE2EStructuredOutput(t *testing.T) {
521522
}
522523
})
523524
}
525+
526+
func TestE2EServerOptions(t *testing.T) {
527+
t.Parallel()
528+
ctx := t.Context()
529+
530+
// Downstream server has 3 tools and 2 resources.
531+
session := startFakeServer(t, ctx, "Original description",
532+
[]mcp.Tool{
533+
{Name: "allowed_tool", Description: "This tool is allowed"},
534+
{Name: "blocked_tool", Description: "This tool is blocked"},
535+
{Name: "another_allowed", Description: "Also allowed"},
536+
},
537+
[]mcp.Resource{
538+
{URI: "file:///allowed.txt", Name: "allowed.txt", Description: "Allowed resource"},
539+
{URI: "file:///blocked.txt", Name: "blocked.txt", Description: "Blocked resource"},
540+
},
541+
)
542+
543+
srv, err := mcpserver.NewServerFromSession(ctx, session, config.ServerOptions{
544+
Description: "Custom skill description",
545+
AllowedTools: []string{"allowed_tool", "another_allowed"},
546+
AllowedResources: []string{"file:///allowed.txt"},
547+
})
548+
if err != nil {
549+
t.Fatal(err)
550+
}
551+
mgr, err := mcpserver.NewManagerFromServers(map[string]*mcpserver.Server{"filtered": srv})
552+
if err != nil {
553+
t.Fatal(err)
554+
}
555+
defer mgr.Close()
556+
557+
upstream := connectTestClient(t, ctx, mgr)
558+
559+
t.Run("list_skills_shows_custom_description", func(t *testing.T) {
560+
result, err := upstream.CallTool(ctx, &mcp.CallToolParams{Name: "list_skills"})
561+
if err != nil {
562+
t.Fatal(err)
563+
}
564+
tc := result.Content[0].(*mcp.TextContent)
565+
if !strings.Contains(tc.Text, "Custom skill description") {
566+
t.Errorf("expected custom description, got %q", tc.Text)
567+
}
568+
if strings.Contains(tc.Text, "Original description") {
569+
t.Error("original description should be overridden")
570+
}
571+
})
572+
573+
t.Run("use_skill_shows_only_allowed_tools", func(t *testing.T) {
574+
result, err := upstream.CallTool(ctx, &mcp.CallToolParams{
575+
Name: "use_skill",
576+
Arguments: map[string]any{"skill_name": "filtered"},
577+
})
578+
if err != nil {
579+
t.Fatal(err)
580+
}
581+
tc := result.Content[0].(*mcp.TextContent)
582+
if !strings.Contains(tc.Text, "allowed_tool(") {
583+
t.Errorf("expected allowed_tool signature, got %q", tc.Text)
584+
}
585+
if !strings.Contains(tc.Text, "another_allowed(") {
586+
t.Errorf("expected another_allowed signature, got %q", tc.Text)
587+
}
588+
if strings.Contains(tc.Text, "blocked_tool") {
589+
t.Error("blocked_tool should not appear")
590+
}
591+
})
592+
593+
t.Run("use_skill_shows_only_allowed_resources", func(t *testing.T) {
594+
result, err := upstream.CallTool(ctx, &mcp.CallToolParams{
595+
Name: "use_skill",
596+
Arguments: map[string]any{"skill_name": "filtered"},
597+
})
598+
if err != nil {
599+
t.Fatal(err)
600+
}
601+
tc := result.Content[0].(*mcp.TextContent)
602+
if !strings.Contains(tc.Text, "file:///allowed.txt") {
603+
t.Errorf("expected allowed resource, got %q", tc.Text)
604+
}
605+
if strings.Contains(tc.Text, "file:///blocked.txt") {
606+
t.Error("blocked resource should not appear")
607+
}
608+
})
609+
610+
t.Run("execute_code_can_call_allowed_tool", func(t *testing.T) {
611+
code := `allowed_tool()`
612+
result, err := upstream.CallTool(ctx, &mcp.CallToolParams{
613+
Name: "execute_code",
614+
Arguments: map[string]any{"code": code},
615+
})
616+
if err != nil {
617+
t.Fatal(err)
618+
}
619+
if result.IsError {
620+
tc := result.Content[0].(*mcp.TextContent)
621+
t.Fatalf("error: %s", tc.Text)
622+
}
623+
})
624+
625+
t.Run("execute_code_cannot_call_blocked_tool", func(t *testing.T) {
626+
code := `blocked_tool()`
627+
result, err := upstream.CallTool(ctx, &mcp.CallToolParams{
628+
Name: "execute_code",
629+
Arguments: map[string]any{"code": code},
630+
})
631+
if err != nil {
632+
t.Fatal(err)
633+
}
634+
if !result.IsError {
635+
t.Error("expected error calling blocked tool")
636+
}
637+
})
638+
639+
t.Run("blocked_tool_does_not_cause_prefix", func(t *testing.T) {
640+
// If blocked_tool were visible, tools with same name in other
641+
// servers would get prefixed. Since it's filtered, no prefix needed.
642+
result, err := upstream.CallTool(ctx, &mcp.CallToolParams{
643+
Name: "use_skill",
644+
Arguments: map[string]any{"skill_name": "filtered"},
645+
})
646+
if err != nil {
647+
t.Fatal(err)
648+
}
649+
tc := result.Content[0].(*mcp.TextContent)
650+
// Tool names should not have "filtered_" prefix since there's no conflict.
651+
if strings.Contains(tc.Text, "filtered_allowed_tool") {
652+
t.Error("tools should not be prefixed when no conflict exists")
653+
}
654+
})
655+
}

internal/mcpserver/manager_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ func TestAllTools(t *testing.T) {
8181
t.Parallel()
8282
ctx := t.Context()
8383

84-
s1, err := NewServerFromSession(ctx, startFakeServer(t, ctx, "tool_a"))
84+
s1, err := NewServerFromSession(ctx, startFakeServer(t, ctx, []string{"tool_a"}, nil))
8585
if err != nil {
8686
t.Fatal(err)
8787
}
88-
s2, err := NewServerFromSession(ctx, startFakeServer(t, ctx, "tool_b"))
88+
s2, err := NewServerFromSession(ctx, startFakeServer(t, ctx, []string{"tool_b"}, nil))
8989
if err != nil {
9090
t.Fatal(err)
9191
}
@@ -104,11 +104,11 @@ func TestManagerServerTools(t *testing.T) {
104104
t.Parallel()
105105
ctx := t.Context()
106106

107-
s1, err := NewServerFromSession(ctx, startFakeServer(t, ctx, "tool_a"))
107+
s1, err := NewServerFromSession(ctx, startFakeServer(t, ctx, []string{"tool_a"}, nil))
108108
if err != nil {
109109
t.Fatal(err)
110110
}
111-
s2, err := NewServerFromSession(ctx, startFakeServer(t, ctx, "tool_b"))
111+
s2, err := NewServerFromSession(ctx, startFakeServer(t, ctx, []string{"tool_b"}, nil))
112112
if err != nil {
113113
t.Fatal(err)
114114
}
@@ -135,7 +135,7 @@ func TestManagerClose(t *testing.T) {
135135
t.Parallel()
136136

137137
ctx := t.Context()
138-
session := startFakeServer(t, ctx, "tool")
138+
session := startFakeServer(t, ctx, []string{"tool"}, nil)
139139
srv, err := NewServerFromSession(ctx, session)
140140
if err != nil {
141141
t.Fatal(err)

0 commit comments

Comments
 (0)