Skip to content

Commit c58fef0

Browse files
dgageotdgageot
authored
Merge pull request #1955 from dgageot/board/are-there-features-in-the-most-recent-mc-7866095f
Leverage latest MCP spec features from go-sdk v1.4.0
2 parents 6621ea9 + e9117f1 commit c58fef0

File tree

5 files changed

+362
-100
lines changed

5 files changed

+362
-100
lines changed

‎pkg/mcp/server.go‎

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,18 +126,18 @@ func createMCPServer(ctx context.Context, agentFilename, agentName string, runCo
126126

127127
slog.Debug("Adding MCP tool", "agent", agentName, "description", description)
128128

129-
readOnly, err := isReadOnlyAgent(ctx, ag)
129+
annotations, err := agentToolAnnotations(ctx, ag)
130130
if err != nil {
131131
cleanup()
132-
return nil, nil, fmt.Errorf("failed to determine if agent %s is read-only: %w", agentName, err)
132+
return nil, nil, fmt.Errorf("failed to compute annotations for agent %s: %w", agentName, err)
133133
}
134134

135+
annotations.Title = description
136+
135137
toolDef := &mcp.Tool{
136-
Name: agentName,
137-
Description: description,
138-
Annotations: &mcp.ToolAnnotations{
139-
ReadOnlyHint: readOnly,
140-
},
138+
Name: agentName,
139+
Description: description,
140+
Annotations: annotations,
141141
InputSchema: tools.MustSchemaFor[ToolInput](),
142142
OutputSchema: tools.MustSchemaFor[ToolOutput](),
143143
}
@@ -185,17 +185,61 @@ func CreateToolHandler(t *team.Team, agentName string) func(context.Context, *mc
185185
}
186186
}
187187

188-
func isReadOnlyAgent(ctx context.Context, ag *agent.Agent) (bool, error) {
188+
// agentToolAnnotations inspects the agent's tools and derives
189+
// [mcp.ToolAnnotations] that describe the aggregate behaviour of the agent.
190+
//
191+
// - ReadOnlyHint is true when every tool is read-only.
192+
// - DestructiveHint is explicitly false when no tool is destructive.
193+
// - IdempotentHint is true when every tool is idempotent.
194+
// - OpenWorldHint is explicitly false when no tool interacts with an open world.
195+
func agentToolAnnotations(ctx context.Context, ag *agent.Agent) (*mcp.ToolAnnotations, error) {
189196
allTools, err := ag.Tools(ctx)
190197
if err != nil {
191-
return false, err
198+
return nil, err
192199
}
193200

201+
readOnly := true
202+
destructive := false
203+
idempotent := true
204+
openWorld := false
205+
194206
for _, tool := range allTools {
195-
if !tool.Annotations.ReadOnlyHint {
196-
return false, nil
207+
a := tool.Annotations
208+
if !a.ReadOnlyHint {
209+
readOnly = false
210+
}
211+
if !a.IdempotentHint {
212+
idempotent = false
197213
}
214+
// *bool hints default to true per the MCP spec; nil means "assumed true".
215+
if optionalBool(a.DestructiveHint, true) {
216+
destructive = true
217+
}
218+
if optionalBool(a.OpenWorldHint, true) {
219+
openWorld = true
220+
}
221+
}
222+
223+
annotations := &mcp.ToolAnnotations{
224+
ReadOnlyHint: readOnly,
225+
IdempotentHint: idempotent,
226+
}
227+
// Only set *bool fields explicitly when they differ from the spec default
228+
// (true), so that nil keeps its "default" semantics on the wire.
229+
if !destructive {
230+
annotations.DestructiveHint = new(bool)
231+
}
232+
if !openWorld {
233+
annotations.OpenWorldHint = new(bool)
198234
}
199235

200-
return true, nil
236+
return annotations, nil
237+
}
238+
239+
// optionalBool returns the value of p, or fallback when p is nil.
240+
func optionalBool(p *bool, fallback bool) bool {
241+
if p != nil {
242+
return *p
243+
}
244+
return fallback
201245
}

‎pkg/mcp/server_test.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package mcp
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
9+
"github.com/docker/cagent/pkg/agent"
10+
"github.com/docker/cagent/pkg/tools"
11+
)
12+
13+
func boolPtr(v bool) *bool { return &v }
14+
15+
// annot is a shorthand for building tools.ToolAnnotations in tests.
16+
func annot(readOnly, idempotent bool, destructive, openWorld *bool) tools.ToolAnnotations {
17+
return tools.ToolAnnotations{
18+
ReadOnlyHint: readOnly,
19+
IdempotentHint: idempotent,
20+
DestructiveHint: destructive,
21+
OpenWorldHint: openWorld,
22+
}
23+
}
24+
25+
func TestAgentToolAnnotations(t *testing.T) {
26+
t.Parallel()
27+
28+
pFalse := boolPtr(false)
29+
pTrue := boolPtr(true)
30+
31+
tests := []struct {
32+
name string
33+
tools []tools.Tool
34+
wantReadOnly bool
35+
wantDestructive *bool // nil means default (true)
36+
wantIdempotent bool
37+
wantOpenWorld *bool // nil means default (true)
38+
}{
39+
{
40+
name: "no tools yields most conservative defaults",
41+
wantReadOnly: true,
42+
wantDestructive: pFalse,
43+
wantIdempotent: true,
44+
wantOpenWorld: pFalse,
45+
},
46+
{
47+
name: "all read-only tools",
48+
tools: []tools.Tool{
49+
{Name: "a", Annotations: annot(true, true, pFalse, pFalse)},
50+
{Name: "b", Annotations: annot(true, true, pFalse, pFalse)},
51+
},
52+
wantReadOnly: true,
53+
wantDestructive: pFalse,
54+
wantIdempotent: true,
55+
wantOpenWorld: pFalse,
56+
},
57+
{
58+
name: "mixed read-only",
59+
tools: []tools.Tool{
60+
{Name: "reader", Annotations: annot(true, false, pFalse, pFalse)},
61+
{Name: "writer", Annotations: annot(false, false, pTrue, pFalse)},
62+
},
63+
wantReadOnly: false,
64+
wantIdempotent: false,
65+
wantOpenWorld: pFalse,
66+
// wantDestructive nil → at least one destructive tool
67+
},
68+
{
69+
name: "nil destructive hint treated as destructive",
70+
tools: []tools.Tool{
71+
{Name: "tool", Annotations: annot(false, false, nil, pFalse)},
72+
},
73+
wantOpenWorld: pFalse,
74+
// wantDestructive nil → nil DestructiveHint defaults to true
75+
},
76+
{
77+
name: "nil open world hint treated as open world",
78+
tools: []tools.Tool{
79+
{Name: "tool", Annotations: annot(false, false, pFalse, nil)},
80+
},
81+
wantDestructive: pFalse,
82+
// wantOpenWorld nil → nil OpenWorldHint defaults to true
83+
},
84+
{
85+
name: "open world tool makes agent open world",
86+
tools: []tools.Tool{
87+
{Name: "closed", Annotations: annot(true, false, pFalse, pFalse)},
88+
{Name: "web", Annotations: annot(true, false, pFalse, pTrue)},
89+
},
90+
wantReadOnly: true,
91+
wantDestructive: pFalse,
92+
// wantOpenWorld nil → open world
93+
},
94+
{
95+
name: "all idempotent",
96+
tools: []tools.Tool{
97+
{Name: "a", Annotations: annot(false, true, pFalse, pFalse)},
98+
{Name: "b", Annotations: annot(false, true, pFalse, pFalse)},
99+
},
100+
wantDestructive: pFalse,
101+
wantIdempotent: true,
102+
wantOpenWorld: pFalse,
103+
},
104+
}
105+
106+
for _, tt := range tests {
107+
t.Run(tt.name, func(t *testing.T) {
108+
t.Parallel()
109+
110+
ag := agent.New("test", "test agent", agent.WithTools(tt.tools...))
111+
got, err := agentToolAnnotations(t.Context(), ag)
112+
require.NoError(t, err)
113+
114+
assert.Equal(t, tt.wantReadOnly, got.ReadOnlyHint, "ReadOnlyHint")
115+
assert.Equal(t, tt.wantDestructive, got.DestructiveHint, "DestructiveHint")
116+
assert.Equal(t, tt.wantIdempotent, got.IdempotentHint, "IdempotentHint")
117+
assert.Equal(t, tt.wantOpenWorld, got.OpenWorldHint, "OpenWorldHint")
118+
})
119+
}
120+
}

‎pkg/tools/mcp/mcp.go‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -488,32 +488,45 @@ func isInitNotificationSendError(err error) bool {
488488
}
489489

490490
func processMCPContent(toolResult *mcp.CallToolResult) *tools.ToolCallResult {
491-
finalContent := ""
492-
var images []tools.ImageContent
491+
var text string
492+
var images, audios []tools.MediaContent
493493

494-
for _, resultContent := range toolResult.Content {
495-
switch c := resultContent.(type) {
494+
for _, c := range toolResult.Content {
495+
switch c := c.(type) {
496496
case *mcp.TextContent:
497-
finalContent += c.Text
497+
text += c.Text
498498
case *mcp.ImageContent:
499-
// MCP SDK decodes the base64 wire format into raw bytes,
500-
// so we need to re-encode to base64 for our ImageContent.
501-
images = append(images, tools.ImageContent{
502-
Data: base64.StdEncoding.EncodeToString(c.Data),
503-
MimeType: c.MIMEType,
504-
})
499+
images = append(images, encodeMedia(c.Data, c.MIMEType))
500+
case *mcp.AudioContent:
501+
audios = append(audios, encodeMedia(c.Data, c.MIMEType))
502+
case *mcp.ResourceLink:
503+
if c.Name != "" {
504+
// Escape ] in name and ) in URI to prevent broken markdown links.
505+
name := strings.ReplaceAll(c.Name, "]", "\\]")
506+
uri := strings.ReplaceAll(c.URI, ")", "%29")
507+
text += fmt.Sprintf("[%s](%s)", name, uri)
508+
} else {
509+
text += c.URI
510+
}
505511
}
506512
}
507513

508-
// Handle an empty response. This can happen if the MCP tool does not return any content.
509-
finalContent = cmp.Or(finalContent, "no output")
514+
return &tools.ToolCallResult{
515+
Output: cmp.Or(text, "no output"),
516+
IsError: toolResult.IsError,
517+
Images: images,
518+
Audios: audios,
519+
StructuredContent: toolResult.StructuredContent,
520+
}
521+
}
510522

511-
result := &tools.ToolCallResult{
512-
Output: finalContent,
513-
IsError: toolResult.IsError,
514-
Images: images,
523+
// encodeMedia re-encodes raw bytes (as decoded by the MCP SDK) back to base64
524+
// for our internal MediaContent representation.
525+
func encodeMedia(data []byte, mimeType string) tools.MediaContent {
526+
return tools.MediaContent{
527+
Data: base64.StdEncoding.EncodeToString(data),
528+
MimeType: mimeType,
515529
}
516-
return result
517530
}
518531

519532
func (ts *Toolset) SetElicitationHandler(handler tools.ElicitationHandler) {

0 commit comments

Comments
 (0)