From a6cede784ad08794bac27d54d2904a915cd76e01 Mon Sep 17 00:00:00 2001 From: ash Date: Sat, 5 Sep 2026 05:00:02 +0000 Subject: [PATCH] client: Honor IsError and extract JSON from tool text Fold Jerry dump into toolJSON: TrimSpace IsError text, concat JSON via rawJSON, and unit tests mirroring IsError=true / StructuredContent=null / Text=`end must be after start` / GetError=nil. --- client/session.go | 32 ++++++++++++-------- client/session_test.go | 68 ++++++++++++++++++++++++++++++++---------- 2 files changed, 72 insertions(+), 28 deletions(-) diff --git a/client/session.go b/client/session.go index 744bf4b..ec01f46 100644 --- a/client/session.go +++ b/client/session.go @@ -100,18 +100,18 @@ func toolJSON(res *mcp.CallToolResult) (json.RawMessage, error) { if res == nil { return nil, fmt.Errorf("empty tool result") } - // GetError is only set by SetError on the server; the err field is not marshaled to clients. + // GetError is set by SetError on the server; the err field is not marshaled to clients. if err := res.GetError(); err != nil { return nil, err } texts, types := collectTextAndTypes(res) concat := strings.Join(texts, "") - // IsError is the client-visible soft-fail flag; error text lives in Content. if res.IsError { - if concat == "" { - return nil, fmt.Errorf("tool error") + msg := strings.TrimSpace(concat) + if msg == "" { + msg = "tool error" } - return nil, fmt.Errorf("%s", concat) + return nil, fmt.Errorf("%s", msg) } if res.StructuredContent != nil { b, err := json.Marshal(res.StructuredContent) @@ -121,16 +121,16 @@ func toolJSON(res *mcp.CallToolResult) (json.RawMessage, error) { return json.RawMessage(b), nil } for _, t := range texts { - if json.Valid([]byte(t)) { - return json.RawMessage(t), nil + if raw, ok := rawJSON(t); ok { + return raw, nil } } + if raw, ok := rawJSON(concat); ok { + return raw, nil + } if concat == "" { return json.RawMessage(`{}`), nil } - if json.Valid([]byte(concat)) { - return json.RawMessage(concat), nil - } if raw := extractFirstJSON(concat); raw != nil { return raw, nil } @@ -138,8 +138,16 @@ func toolJSON(res *mcp.CallToolResult) (json.RawMessage, error) { if res.StructuredContent != nil { structured = "present" } - return nil, fmt.Errorf("non-json tool result (isError=%v types=[%s] structured=%s textLen=%d prefix=%q)", - res.IsError, strings.Join(types, " "), structured, utf8.RuneCountInString(concat), runePrefix(concat, 80)) + return nil, fmt.Errorf("non-json tool result (isError=%v types=%v structured=%s textLen=%d prefix=%q)", + res.IsError, types, structured, utf8.RuneCountInString(concat), runePrefix(concat, 80)) +} + +func rawJSON(s string) (json.RawMessage, bool) { + b := []byte(s) + if !json.Valid(b) { + return nil, false + } + return json.RawMessage(b), true } func collectTextAndTypes(res *mcp.CallToolResult) (texts, types []string) { diff --git a/client/session_test.go b/client/session_test.go index 079a76c..de18027 100644 --- a/client/session_test.go +++ b/client/session_test.go @@ -46,7 +46,7 @@ func TestToolJSON(t *testing.T) { { name: "wrapped JSON in prose", res: &mcp.CallToolResult{ - Content: []mcp.Content{&mcp.TextContent{Text: `here is data: {"symbol":"MU"} thanks`}}, + Content: []mcp.Content{&mcp.TextContent{Text: `here: {"symbol":"MU"} ok`}}, }, want: `{"symbol":"MU"}`, }, @@ -60,6 +60,16 @@ func TestToolJSON(t *testing.T) { }, want: `{"symbol":"MU"}`, }, + { + name: "concatenated text JSON", + res: &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: `{"symbol":`}, + &mcp.TextContent{Text: `"MU"}`}, + }, + }, + want: `{"symbol":"MU"}`, + }, { name: "wrapped JSON array", res: &mcp.CallToolResult{ @@ -110,18 +120,38 @@ func TestToolJSON_getError(t *testing.T) { func TestToolJSON_isErrorText(t *testing.T) { t.Parallel() + // Jerry dump: IsError=true, StructuredContent=null, Text=`end must be after start`, GetError=nil. res := &mcp.CallToolResult{ - IsError: true, - Content: []mcp.Content{&mcp.TextContent{Text: "historicals unavailable"}}, + IsError: true, + StructuredContent: nil, + Content: []mcp.Content{&mcp.TextContent{Text: "end must be after start"}}, } if res.GetError() != nil { - t.Fatal("GetError should be nil when IsError is set via field") + t.Fatal("GetError should be nil on a client-shaped result") } _, err := toolJSON(res) if err == nil { t.Fatal("expected error") } - if err.Error() != "historicals unavailable" { + if err.Error() != "end must be after start" { + t.Fatalf("%v", err) + } + if strings.Contains(err.Error(), "non-json tool result") { + t.Fatalf("got non-json wrapping: %v", err) + } +} + +func TestToolJSON_isErrorOverStructured(t *testing.T) { + t.Parallel() + _, err := toolJSON(&mcp.CallToolResult{ + IsError: true, + StructuredContent: map[string]any{"symbol": "MU"}, + Content: []mcp.Content{&mcp.TextContent{Text: "end must be after start"}}, + }) + if err == nil { + t.Fatal("expected error") + } + if err.Error() != "end must be after start" { t.Fatalf("%v", err) } if strings.Contains(err.Error(), "non-json tool result") { @@ -131,15 +161,21 @@ func TestToolJSON_isErrorText(t *testing.T) { func TestToolJSON_isErrorEmpty(t *testing.T) { t.Parallel() - _, err := toolJSON(&mcp.CallToolResult{IsError: true}) - if err == nil { - t.Fatal("expected error") + tests := []*mcp.CallToolResult{ + {IsError: true}, + {IsError: true, Content: []mcp.Content{&mcp.TextContent{Text: " \n"}}}, } - if err.Error() != "tool error" { - t.Fatalf("%v", err) - } - if strings.Contains(err.Error(), "non-json tool result") { - t.Fatalf("got non-json wrapping: %v", err) + for _, res := range tests { + _, err := toolJSON(res) + if err == nil { + t.Fatal("expected error") + } + if err.Error() != "tool error" { + t.Fatalf("%v", err) + } + if strings.Contains(err.Error(), "non-json tool result") { + t.Fatalf("got non-json wrapping: %v", err) + } } } @@ -152,10 +188,10 @@ func TestToolJSON_nonJSONText(t *testing.T) { t.Fatal("expected error") } msg := err.Error() - if !strings.Contains(msg, "non-json") { - t.Fatalf("missing non-json: %v", err) + if !strings.Contains(msg, "non-json tool result") { + t.Fatalf("missing non-json tool result: %v", err) } - for _, want := range []string{`isError=false`, `types=[text]`, `structured=nil`, `prefix="not json"`} { + for _, want := range []string{`isError=false`, `types=[text]`, `structured=nil`, `textLen=8`, `prefix="not json"`} { if !strings.Contains(msg, want) { t.Fatalf("missing %q in %v", want, err) }