Files
cogito/mcp.go
Ettore Di Giacinto f2c20252ba fix(mcp): handle non-text tool result content without panicking (#50)
* fix(mcp): handle non-text tool result content without panicking

An MCP tool result is a slice of mcp.Content, which may contain image,
audio, resource-link or embedded-resource blocks in addition to text.
mcpTool.Execute asserted every block to *mcp.TextContent unconditionally,
so any tool returning media (e.g. osmmcp get_map_image, NASA image tools)
triggered:

  panic: interface conversion: mcp.Content is *mcp.ImageContent, not *mcp.TextContent

The panic was on an agent goroutine with no recover, taking the whole
host process down (mudler/LocalAI#10101).

Extract a contentToString helper that type-switches over every mcp.Content
variant: text is concatenated verbatim (preserving prior behavior), media
and resource blocks are summarized with a descriptive marker, and unknown
types are logged instead of crashing.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* fix(tools): never send consecutive assistant messages to the backend

cogito's tool loop appends an assistant "reasoning" message (toolSelection
no-tool / no-tool-selected paths) on top of a fragment that may already end
with an assistant message. The resulting decision request then ends with two
assistant messages in a row, which llama.cpp via LocalAI rejects:

  500 InvalidArgument: Cannot have 2 or more assistant messages at the end of the list

Because decisionWithStreaming retries the identical request, all attempts hit
the same error and the whole reviewer/tool flow fails (the Test E2E
"refine a content with a search tool" spec, and any agent feeding an
assistant-terminated fragment into a tool loop).

Add mergeConsecutiveAssistantMessages, applied alongside normalizeSystemMessages
at the single boundary where both decision() and decisionWithStreaming() build
the request. It collapses any run of consecutive assistant messages into one,
concatenating content and preserving tool calls, so the list can never end with
two assistant messages.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* test(e2e): compare tool-call arguments by JSON value, not raw bytes

The "should select a tool" spec re-marshaled the parsed arguments map to
compact JSON and compared it byte-for-byte against the tool-call message's
Arguments via HaveExactElements. The message preserves the model's raw
arguments string, whose whitespace differs ({"city": "San Francisco"} vs
{"city":"San Francisco"}), so the assertion failed deterministically
(all 5 flake attempts) even though the tool and arguments were correct.

Assert length, type and name explicitly and match the arguments with
MatchJSON, which compares JSON semantically and ignores insignificant
whitespace and key ordering.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

---------

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-05-31 10:53:16 +02:00

299 lines
8.4 KiB
Go

package cogito
import (
"context"
"encoding/json"
"errors"
"fmt"
"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/mudler/xlog"
"github.com/sashabaranov/go-openai"
"github.com/tmc/langchaingo/jsonschema"
)
type mcpTool struct {
name, description string
inputSchema toolInputSchema
session *mcp.ClientSession
ctx context.Context
props map[string]jsonschema.Definition
}
func (t *mcpTool) Tool() openai.Tool {
return openai.Tool{
Type: openai.ToolTypeFunction,
Function: &openai.FunctionDefinition{
Name: t.name,
Description: t.description,
Parameters: jsonschema.Definition{
Type: jsonschema.Object,
Properties: t.props,
Required: t.inputSchema.Required,
},
},
}
}
func (t *mcpTool) Execute(args map[string]any) (string, any, error) {
// Call a tool on the server.
params := &mcp.CallToolParams{
Name: t.name,
Arguments: args,
}
res, err := t.session.CallTool(t.ctx, params)
if err != nil {
xlog.Error("CallTool failed: %v", err)
return "", nil, err
}
result := contentToString(res.Content)
if res.IsError {
xlog.Error("tool failed", "result", result)
return result, nil, errors.New("tool failed: " + result)
}
return result, res, nil
}
// contentToString flattens the content blocks of an MCP tool result into a
// single textual representation that can be fed back to the model. Non-text
// blocks (images, audio, resources) are summarized with a descriptive marker
// instead of being asserted to *mcp.TextContent, which would panic and crash
// the host process when a tool returns media (see mudler/LocalAI#10101).
func contentToString(content []mcp.Content) string {
result := ""
for _, c := range content {
switch v := c.(type) {
case *mcp.TextContent:
result += v.Text
case *mcp.ImageContent:
result += fmt.Sprintf("[image content (%s), %d bytes]", v.MIMEType, len(v.Data))
case *mcp.AudioContent:
result += fmt.Sprintf("[audio content (%s), %d bytes]", v.MIMEType, len(v.Data))
case *mcp.ResourceLink:
result += fmt.Sprintf("[resource link: %s]", v.URI)
case *mcp.EmbeddedResource:
switch {
case v.Resource == nil:
result += "[embedded resource]"
case v.Resource.Text != "":
result += v.Resource.Text
default:
result += fmt.Sprintf("[embedded resource: %s]", v.Resource.URI)
}
default:
xlog.Warn("Unhandled MCP content type", "type", fmt.Sprintf("%T", c))
}
}
return result
}
func (t *mcpTool) Close() {
if err := t.session.Close(); err != nil {
xlog.Warn("Failed to close MCP session", "error", err)
}
}
type toolInputSchema struct {
Type string `json:"type"`
Properties map[string]interface{} `json:"properties,omitempty"`
Required []string `json:"required,omitempty"`
}
// CoerceNullableTypes is an exported alias for the same workaround so
// downstream tests can verify their own MCP servers stay compatible
// with the import path. Most callers won't need it.
func CoerceNullableTypes(props map[string]any) { coerceNullableTypes(props) }
// coerceNullableTypes recursively walks a property bag and rewrites any
// JSON-Schema 2020-12 "type": ["null", "X"] into "type": "X". The
// downstream langchaingo/jsonschema.Definition we unmarshal into has
// Type as a single string, so an unflattened type-array would fail
// the unmarshal and silently drop the tool. Picks the first non-null
// member; falls back to the first member if all are null.
//
// Recurses into every nested schema location a `type` field can
// appear: properties, items, oneOf/anyOf/allOf members, $defs/
// definitions, additionalProperties, patternProperties.
func coerceNullableTypes(props map[string]any) {
if props == nil {
return
}
for _, raw := range props {
coerceSchema(raw)
}
}
// coerceSchema applies the type-array → string rewrite to a single
// schema node, then recurses into every nested schema location.
func coerceSchema(node any) {
obj, ok := node.(map[string]any)
if !ok {
return
}
if t, ok := obj["type"].([]any); ok {
pick := ""
for _, m := range t {
s, ok := m.(string)
if !ok || s == "null" {
continue
}
pick = s
break
}
if pick == "" && len(t) > 0 {
if s, ok := t[0].(string); ok {
pick = s
}
}
if pick != "" {
obj["type"] = pick
}
}
// Object properties — map of name → schema.
if nested, ok := obj["properties"].(map[string]any); ok {
coerceNullableTypes(nested)
}
// patternProperties — same shape as properties, just regex-keyed.
if nested, ok := obj["patternProperties"].(map[string]any); ok {
coerceNullableTypes(nested)
}
// $defs / definitions — JSON-Schema named schema bag.
if nested, ok := obj["$defs"].(map[string]any); ok {
coerceNullableTypes(nested)
}
if nested, ok := obj["definitions"].(map[string]any); ok {
coerceNullableTypes(nested)
}
// Single nested schema fields.
coerceSchema(obj["items"])
coerceSchema(obj["additionalProperties"])
coerceSchema(obj["contains"])
coerceSchema(obj["not"])
coerceSchema(obj["if"])
coerceSchema(obj["then"])
coerceSchema(obj["else"])
coerceSchema(obj["propertyNames"])
// Composition keywords — arrays of schemas.
for _, key := range []string{"oneOf", "anyOf", "allOf"} {
arr, ok := obj[key].([]any)
if !ok {
continue
}
for _, member := range arr {
coerceSchema(member)
}
}
// "items" can also be an array (tuple validation in older drafts).
if arr, ok := obj["items"].([]any); ok {
for _, member := range arr {
coerceSchema(member)
}
}
// "prefixItems" (2020-12 tuple).
if arr, ok := obj["prefixItems"].([]any); ok {
for _, member := range arr {
coerceSchema(member)
}
}
}
func mcpPromptsFromTransport(ctx context.Context, session *mcp.ClientSession, arguments map[string]string) ([]openai.ChatCompletionMessage, error) {
prompts, err := session.ListPrompts(ctx, nil)
if err != nil {
return nil, err
}
promptsList := []openai.ChatCompletionMessage{}
for _, prompt := range prompts.Prompts {
p, err := session.GetPrompt(ctx, &mcp.GetPromptParams{Name: prompt.Name, Arguments: arguments})
if err != nil {
return nil, err
}
for _, message := range p.Messages {
switch message.Content.(type) {
case *mcp.TextContent:
promptsList = append(promptsList, openai.ChatCompletionMessage{
Role: string(message.Role),
Content: message.Content.(*mcp.TextContent).Text,
})
}
}
}
return promptsList, nil
}
// MCPToolFilter is invoked once per (session, tool) pair during the
// initial tool-discovery pass. Return false to drop the tool from the
// agent's discovered set (the LLM never sees it). A nil filter is
// equivalent to "always allow".
type MCPToolFilter = func(session *mcp.ClientSession, toolName string) bool
// probe the MCP remote and generate tools that are compliant with cogito
func mcpToolsFromTransport(ctx context.Context, session *mcp.ClientSession, filter MCPToolFilter) ([]ToolDefinitionInterface, error) {
allTools := []ToolDefinitionInterface{}
tools, err := session.ListTools(ctx, nil)
if err != nil {
xlog.Error("Error listing tools: %v", err)
return nil, err
}
for _, tool := range tools.Tools {
if filter != nil && !filter(session, tool.Name) {
continue
}
dat, err := json.Marshal(tool.InputSchema)
if err != nil {
xlog.Error("Error marshalling input schema: %v", err)
continue
}
var inputSchema toolInputSchema
err = json.Unmarshal(dat, &inputSchema)
if err != nil {
xlog.Error("Error unmarshalling input schema: %v", err)
continue
}
// Some MCP servers (e.g. modelcontextprotocol/go-sdk v1.4+)
// emit JSON Schema 2020-12 "type": ["null", "array"] for
// nullable fields like Go []string slices. langchaingo's
// jsonschema.Definition.Type is a single string, so the
// unmarshal would fail and silently drop the entire tool.
// Coerce any type-array to its non-null string member before
// unmarshalling so those tools stay discoverable.
coerceNullableTypes(inputSchema.Properties)
props := map[string]jsonschema.Definition{}
dat, err = json.Marshal(inputSchema.Properties)
if err != nil {
xlog.Error("Error marshalling input schema: %v", err)
continue
}
err = json.Unmarshal(dat, &props)
if err != nil {
xlog.Error("Error unmarshalling input schema properties: %v", err)
continue
}
allTools = append(allTools, &mcpTool{
name: tool.Name,
description: tool.Description,
session: session,
ctx: ctx,
props: props,
inputSchema: inputSchema,
})
}
return allTools, nil
}