Skip to content

Commit eac0ea7

Browse files
feat: Add NewToolsetGroupFromTools constructor
Add a new constructor that creates a ToolsetGroup from a list of ServerTools. Tools are automatically categorized as read or write based on their ReadOnlyHint annotation, and grouped into toolsets based on the 'toolset' field in their Meta. This enables tools to be fully self-describing - registration can now derive toolset membership and read/write classification directly from the tool definition.
1 parent c6f31f3 commit eac0ea7

File tree

2 files changed

+275
-0
lines changed

2 files changed

+275
-0
lines changed

pkg/toolsets/toolsets.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,75 @@ func NewToolsetGroup(readOnly bool) *ToolsetGroup {
207207
}
208208
}
209209

210+
// ToolsetInfo holds the name and description for a toolset
211+
type ToolsetInfo struct {
212+
Name string
213+
Description string
214+
}
215+
216+
// NewToolsetGroupFromTools creates a ToolsetGroup from a list of ServerTools.
217+
// Tools are automatically categorized as read or write based on their ReadOnlyHint annotation.
218+
// Tools are grouped into toolsets based on the "toolset" field in their Meta.
219+
// The toolsetInfos map provides descriptions for each toolset name.
220+
func NewToolsetGroupFromTools(readOnly bool, toolsetInfos map[string]ToolsetInfo, tools ...ServerTool) *ToolsetGroup {
221+
tsg := NewToolsetGroup(readOnly)
222+
223+
// Group tools by toolset name
224+
toolsByToolset := make(map[string][]ServerTool)
225+
for _, tool := range tools {
226+
toolsetName := getToolsetFromMeta(tool.Tool.Meta)
227+
if toolsetName == "" {
228+
panic(fmt.Sprintf("tool %q has no toolset in Meta", tool.Tool.Name))
229+
}
230+
toolsByToolset[toolsetName] = append(toolsByToolset[toolsetName], tool)
231+
}
232+
233+
// Create toolsets and add tools
234+
for toolsetName, toolsetTools := range toolsByToolset {
235+
info, ok := toolsetInfos[toolsetName]
236+
if !ok {
237+
// Use a default description if not provided
238+
info = ToolsetInfo{Name: toolsetName, Description: ""}
239+
}
240+
241+
ts := NewToolset(info.Name, info.Description)
242+
243+
for _, tool := range toolsetTools {
244+
if isReadOnlyTool(tool) {
245+
ts.readTools = append(ts.readTools, tool)
246+
} else {
247+
ts.writeTools = append(ts.writeTools, tool)
248+
}
249+
}
250+
251+
tsg.AddToolset(ts)
252+
}
253+
254+
return tsg
255+
}
256+
257+
// getToolsetFromMeta extracts the toolset name from tool metadata
258+
func getToolsetFromMeta(meta mcp.Meta) string {
259+
if meta == nil {
260+
return ""
261+
}
262+
if toolset, ok := meta["toolset"].(string); ok {
263+
return toolset
264+
}
265+
return ""
266+
}
267+
268+
// isReadOnlyTool determines if a tool is read-only based on its annotations.
269+
// A tool is considered read-only only if ReadOnlyHint is explicitly true.
270+
// Write tools have ReadOnlyHint=false or DestructiveHint=true.
271+
func isReadOnlyTool(tool ServerTool) bool {
272+
if tool.Tool.Annotations == nil {
273+
// No annotations means we assume it could write (worst case)
274+
return false
275+
}
276+
return tool.Tool.Annotations.ReadOnlyHint
277+
}
278+
210279
func (tg *ToolsetGroup) AddDeprecatedToolAliases(aliases map[string]string) {
211280
for oldName, newName := range aliases {
212281
tg.deprecatedAliases[oldName] = newName

pkg/toolsets/toolsets_test.go

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,3 +393,209 @@ func TestRegisterSpecificTools(t *testing.T) {
393393
t.Error("expected error for non-existent tool")
394394
}
395395
}
396+
397+
// mockToolWithMeta creates a ServerTool with metadata for testing NewToolsetGroupFromTools
398+
func mockToolWithMeta(name string, toolsetName string, readOnly bool) ServerTool {
399+
return ServerTool{
400+
Tool: mcp.Tool{
401+
Name: name,
402+
Meta: mcp.Meta{"toolset": toolsetName},
403+
Annotations: &mcp.ToolAnnotations{
404+
ReadOnlyHint: readOnly,
405+
},
406+
},
407+
RegisterFunc: func(_ *mcp.Server) {},
408+
}
409+
}
410+
411+
func TestNewToolsetGroupFromTools(t *testing.T) {
412+
toolsetInfos := map[string]ToolsetInfo{
413+
"repos": {Name: "repos", Description: "Repository tools"},
414+
"issues": {Name: "issues", Description: "Issue tools"},
415+
}
416+
417+
// Create tools with meta containing toolset info
418+
tools := []ServerTool{
419+
mockToolWithMeta("get_repo", "repos", true),
420+
mockToolWithMeta("create_repo", "repos", false),
421+
mockToolWithMeta("get_issue", "issues", true),
422+
mockToolWithMeta("create_issue", "issues", false),
423+
mockToolWithMeta("list_issues", "issues", true),
424+
}
425+
426+
tsg := NewToolsetGroupFromTools(false, toolsetInfos, tools...)
427+
428+
// Verify toolsets were created
429+
if len(tsg.Toolsets) != 2 {
430+
t.Fatalf("expected 2 toolsets, got %d", len(tsg.Toolsets))
431+
}
432+
433+
// Verify repos toolset
434+
reposToolset, exists := tsg.Toolsets["repos"]
435+
if !exists {
436+
t.Fatal("expected 'repos' toolset to exist")
437+
}
438+
if reposToolset.Description != "Repository tools" {
439+
t.Errorf("expected repos description 'Repository tools', got '%s'", reposToolset.Description)
440+
}
441+
if len(reposToolset.readTools) != 1 {
442+
t.Errorf("expected 1 read tool in repos, got %d", len(reposToolset.readTools))
443+
}
444+
if len(reposToolset.writeTools) != 1 {
445+
t.Errorf("expected 1 write tool in repos, got %d", len(reposToolset.writeTools))
446+
}
447+
448+
// Verify issues toolset
449+
issuesToolset, exists := tsg.Toolsets["issues"]
450+
if !exists {
451+
t.Fatal("expected 'issues' toolset to exist")
452+
}
453+
if len(issuesToolset.readTools) != 2 {
454+
t.Errorf("expected 2 read tools in issues, got %d", len(issuesToolset.readTools))
455+
}
456+
if len(issuesToolset.writeTools) != 1 {
457+
t.Errorf("expected 1 write tool in issues, got %d", len(issuesToolset.writeTools))
458+
}
459+
}
460+
461+
func TestNewToolsetGroupFromToolsReadOnly(t *testing.T) {
462+
toolsetInfos := map[string]ToolsetInfo{
463+
"repos": {Name: "repos", Description: "Repository tools"},
464+
}
465+
466+
tools := []ServerTool{
467+
mockToolWithMeta("get_repo", "repos", true),
468+
mockToolWithMeta("create_repo", "repos", false),
469+
}
470+
471+
// Create with readOnly=true
472+
tsg := NewToolsetGroupFromTools(true, toolsetInfos, tools...)
473+
474+
reposToolset := tsg.Toolsets["repos"]
475+
if !reposToolset.readOnly {
476+
t.Error("expected toolset to be in read-only mode")
477+
}
478+
479+
// GetActiveTools should only return read tools
480+
activeTools := reposToolset.GetActiveTools()
481+
if len(activeTools) != 0 {
482+
// Toolset is not enabled yet
483+
t.Errorf("expected 0 active tools (not enabled), got %d", len(activeTools))
484+
}
485+
486+
reposToolset.Enabled = true
487+
activeTools = reposToolset.GetActiveTools()
488+
if len(activeTools) != 1 {
489+
t.Errorf("expected 1 active tool in read-only mode, got %d", len(activeTools))
490+
}
491+
if activeTools[0].Tool.Name != "get_repo" {
492+
t.Errorf("expected only read tool 'get_repo', got '%s'", activeTools[0].Tool.Name)
493+
}
494+
}
495+
496+
func TestNewToolsetGroupFromToolsPanicsOnMissingToolset(t *testing.T) {
497+
defer func() {
498+
if r := recover(); r == nil {
499+
t.Error("expected panic when tool has no toolset in Meta")
500+
}
501+
}()
502+
503+
// Tool without toolset in meta
504+
tools := []ServerTool{
505+
{
506+
Tool: mcp.Tool{
507+
Name: "bad_tool",
508+
Meta: nil, // No meta
509+
Annotations: &mcp.ToolAnnotations{
510+
ReadOnlyHint: true,
511+
},
512+
},
513+
RegisterFunc: func(_ *mcp.Server) {},
514+
},
515+
}
516+
517+
NewToolsetGroupFromTools(false, nil, tools...)
518+
}
519+
520+
func TestIsReadOnlyTool(t *testing.T) {
521+
tests := []struct {
522+
name string
523+
tool ServerTool
524+
expected bool
525+
}{
526+
{
527+
name: "read-only tool",
528+
tool: ServerTool{
529+
Tool: mcp.Tool{
530+
Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true},
531+
},
532+
},
533+
expected: true,
534+
},
535+
{
536+
name: "write tool",
537+
tool: ServerTool{
538+
Tool: mcp.Tool{
539+
Annotations: &mcp.ToolAnnotations{ReadOnlyHint: false},
540+
},
541+
},
542+
expected: false,
543+
},
544+
{
545+
name: "no annotations (assume write)",
546+
tool: ServerTool{
547+
Tool: mcp.Tool{
548+
Annotations: nil,
549+
},
550+
},
551+
expected: false,
552+
},
553+
}
554+
555+
for _, tt := range tests {
556+
t.Run(tt.name, func(t *testing.T) {
557+
result := isReadOnlyTool(tt.tool)
558+
if result != tt.expected {
559+
t.Errorf("expected %v, got %v", tt.expected, result)
560+
}
561+
})
562+
}
563+
}
564+
565+
func TestGetToolsetFromMeta(t *testing.T) {
566+
tests := []struct {
567+
name string
568+
meta mcp.Meta
569+
expected string
570+
}{
571+
{
572+
name: "valid toolset",
573+
meta: mcp.Meta{"toolset": "repos"},
574+
expected: "repos",
575+
},
576+
{
577+
name: "nil meta",
578+
meta: nil,
579+
expected: "",
580+
},
581+
{
582+
name: "missing toolset key",
583+
meta: mcp.Meta{"other": "value"},
584+
expected: "",
585+
},
586+
{
587+
name: "wrong type for toolset",
588+
meta: mcp.Meta{"toolset": 123},
589+
expected: "",
590+
},
591+
}
592+
593+
for _, tt := range tests {
594+
t.Run(tt.name, func(t *testing.T) {
595+
result := getToolsetFromMeta(tt.meta)
596+
if result != tt.expected {
597+
t.Errorf("expected %q, got %q", tt.expected, result)
598+
}
599+
})
600+
}
601+
}

0 commit comments

Comments
 (0)