diff --git a/pkg/api/toolsets.go b/pkg/api/toolsets.go index 1f11de579..53907f6f7 100644 --- a/pkg/api/toolsets.go +++ b/pkg/api/toolsets.go @@ -66,9 +66,18 @@ type ToolCallResult struct { // NewToolCallResult creates a ToolCallResult with text content only. // Use this for tools that return human-readable text output. func NewToolCallResult(content string, err error) *ToolCallResult { + return NewToolCallResultFull(content, nil, err) +} + +// NewToolCallResultFull creates a ToolCallResult with both human-readable text +// and structured content. +// Use this when the text representation differs from a JSON serialization of the +// structured content (e.g., YAML or Table text alongside extracted structured data). +func NewToolCallResultFull(text string, structured any, err error) *ToolCallResult { return &ToolCallResult{ - Content: content, - Error: err, + Content: text, + StructuredContent: structured, + Error: err, } } @@ -90,11 +99,7 @@ func NewToolCallResultStructured(structured any, err error) *ToolCallResult { content = string(b) } } - return &ToolCallResult{ - Content: content, - StructuredContent: structured, - Error: err, - } + return NewToolCallResultFull(content, structured, err) } type ToolHandlerParams struct { diff --git a/pkg/api/toolsets_test.go b/pkg/api/toolsets_test.go index e98e579d3..59a5c514f 100644 --- a/pkg/api/toolsets_test.go +++ b/pkg/api/toolsets_test.go @@ -103,6 +103,37 @@ func (s *ToolsetsSuite) TestNewToolCallResultStructured() { }) } +func (s *ToolsetsSuite) TestNewToolCallResultFull() { + s.Run("sets text, structured, and nil error", func() { + structured := []map[string]any{{"name": "pod-1"}} + result := NewToolCallResultFull("formatted text", structured, nil) + s.Equal("formatted text", result.Content) + s.Equal(structured, result.StructuredContent) + s.Nil(result.Error) + }) + s.Run("sets text, structured, and error", func() { + err := errors.New("partial failure") + structured := map[string]any{"key": "value"} + result := NewToolCallResultFull("some text", structured, err) + s.Equal("some text", result.Content) + s.Equal(structured, result.StructuredContent) + s.Equal(err, result.Error) + }) + s.Run("preserves human-readable text separate from structured data", func() { + structured := []map[string]any{{"Name": "ns-1"}, {"Name": "ns-2"}} + result := NewToolCallResultFull("NAMESPACE AGE\nns-1 10d\nns-2 5d", structured, nil) + s.Contains(result.Content, "NAMESPACE") + items, ok := result.StructuredContent.([]map[string]any) + s.Require().True(ok) + s.Len(items, 2) + }) + s.Run("allows nil structured content", func() { + result := NewToolCallResultFull("text only", nil, nil) + s.Equal("text only", result.Content) + s.Nil(result.StructuredContent) + }) +} + func (s *ToolsetsSuite) TestToolMeta() { s.Run("Meta is omitted from JSON when nil", func() { tool := Tool{Name: "test_tool"} diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index 58306cb54..7715deba0 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "os" + "reflect" "slices" "sync" "time" @@ -399,6 +400,10 @@ func NewTextResult(content string, err error) *mcp.CallToolResult { // The Content field contains the JSON-serialized form of structuredContent // for backward compatibility with MCP clients that don't support structuredContent. // +// Per the MCP specification, structuredContent must marshal to a JSON object. +// If structuredContent is a slice/array, it is automatically wrapped in +// {"items": [...]} to satisfy this requirement. +// // Per the MCP specification: // "For backwards compatibility, a tool that returns structured content SHOULD // also return the serialized JSON in a TextContent block." @@ -425,7 +430,26 @@ func NewStructuredResult(content string, structuredContent any, err error) *mcp. }, } if structuredContent != nil { - result.StructuredContent = structuredContent + result.StructuredContent = ensureStructuredObject(structuredContent) } return result } + +// ensureStructuredObject wraps slice/array values in a {"items": ...} object +// because the MCP specification requires structuredContent to be a JSON object. +// A typed nil slice (e.g. []string(nil)) returns nil to avoid {"items": null}. +// Note: this checks the top-level reflect.Kind, so a pointer-to-slice (*[]T) +// would not be wrapped. All current callers pass value types. +func ensureStructuredObject(v any) any { + rv := reflect.ValueOf(v) + if rv.Kind() == reflect.Slice { + if rv.IsNil() { + return nil + } + return map[string]any{"items": v} + } + if rv.Kind() == reflect.Array { + return map[string]any{"items": v} + } + return v +} diff --git a/pkg/mcp/text_result_test.go b/pkg/mcp/text_result_test.go index 884165bfb..ce63fef06 100644 --- a/pkg/mcp/text_result_test.go +++ b/pkg/mcp/text_result_test.go @@ -48,6 +48,24 @@ func (s *TextResultSuite) TestNewStructuredResult() { s.Equal(`{"pods":["pod-1","pod-2"]}`, tc.Text) s.Equal(structured, result.StructuredContent) }) + s.Run("wraps slice in object for MCP spec compliance", func() { + items := []map[string]any{{"name": "ns-1"}, {"name": "ns-2"}} + result := NewStructuredResult("text", items, nil) + s.False(result.IsError) + wrapped, ok := result.StructuredContent.(map[string]any) + s.Require().True(ok, "expected map[string]any wrapper") + s.Equal(items, wrapped["items"]) + }) + s.Run("does not wrap map structured content", func() { + structured := map[string]any{"key": "value"} + result := NewStructuredResult("text", structured, nil) + s.Equal(structured, result.StructuredContent) + }) + s.Run("omits structured content for typed nil slice", func() { + var items []map[string]any // typed nil + result := NewStructuredResult("text", items, nil) + s.Nil(result.StructuredContent, "typed nil slice should not produce {\"items\": null}") + }) s.Run("omits structured content when nil", func() { result := NewStructuredResult("text output", nil, nil) s.False(result.IsError) diff --git a/pkg/output/output.go b/pkg/output/output.go index c558ae9df..433584d04 100644 --- a/pkg/output/output.go +++ b/pkg/output/output.go @@ -14,6 +14,18 @@ var Yaml = &yaml{} var Table = &table{} +// PrintResult holds both the text representation and optional structured data +// extracted from a Kubernetes object. +type PrintResult struct { + // Text is the human-readable formatted output (YAML or Table). + Text string + // Structured is an optional JSON-serializable value extracted from the object. + // For Table output, this is []map[string]any with column headers as keys. + // For YAML output, this is the cleaned-up object items as []map[string]any (lists) + // or a single map[string]any (individual objects). + Structured any +} + type Output interface { // GetName returns the name of the output format, will be used by the CLI to identify the output format. GetName() string @@ -21,6 +33,8 @@ type Output interface { AsTable() bool // PrintObj prints the given object as a string. PrintObj(obj runtime.Unstructured) (string, error) + // PrintObjStructured prints the given object and also extracts structured data. + PrintObjStructured(obj runtime.Unstructured) (*PrintResult, error) } var Outputs = []Output{ @@ -50,6 +64,23 @@ func (p *yaml) AsTable() bool { func (p *yaml) PrintObj(obj runtime.Unstructured) (string, error) { return MarshalYaml(obj) } +func (p *yaml) PrintObjStructured(obj runtime.Unstructured) (*PrintResult, error) { + text, err := p.PrintObj(obj) + if err != nil { + return nil, err + } + switch t := obj.(type) { + case *unstructured.UnstructuredList: + items := make([]map[string]any, 0, len(t.Items)) + for _, item := range t.Items { + items = append(items, item.DeepCopy().Object) + } + return &PrintResult{Text: text, Structured: items}, nil + case *unstructured.Unstructured: + return &PrintResult{Text: text, Structured: t.DeepCopy().Object}, nil + } + return &PrintResult{Text: text}, nil +} type table struct{} @@ -60,12 +91,34 @@ func (p *table) AsTable() bool { return true } func (p *table) PrintObj(obj runtime.Unstructured) (string, error) { + text, _, err := p.printTable(obj) + return text, err +} + +func (p *table) PrintObjStructured(obj runtime.Unstructured) (*PrintResult, error) { + text, t, err := p.printTable(obj) + if err != nil { + return nil, err + } + // Guard against typed nil leaking into the any interface — a nil []map[string]any + // assigned to Structured (type any) would create a non-nil interface, causing + // downstream nil checks (e.g. in NewStructuredResult) to incorrectly pass. + if structured := tableToStructured(t); structured != nil { + return &PrintResult{Text: text, Structured: structured}, nil + } + return &PrintResult{Text: text}, nil +} + +// printTable formats the object as a table and returns the text, the parsed Table (if available), and any error. +func (p *table) printTable(obj runtime.Unstructured) (string, *metav1.Table, error) { var objectToPrint runtime.Object = obj + var parsedTable *metav1.Table withNamespace := false if obj.GetObjectKind().GroupVersionKind() == metav1.SchemeGroupVersion.WithKind("Table") { t := &metav1.Table{} if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), t); err == nil { objectToPrint = t + parsedTable = t // Process the Raw object to retrieve the complete metadata (see kubectl/pkg/printers/table_printer.go) for i := range t.Rows { row := &t.Rows[i] @@ -92,7 +145,34 @@ func (p *table) PrintObj(obj runtime.Unstructured) (string, error) { ShowLabels: true, }) err := printer.PrintObj(objectToPrint, buf) - return buf.String(), err + return buf.String(), parsedTable, err +} + +// tableToStructured converts a Kubernetes Table response to []map[string]any +// using column definitions as keys. +func tableToStructured(t *metav1.Table) []map[string]any { + if t == nil || len(t.Rows) == 0 { + return nil + } + result := make([]map[string]any, 0, len(t.Rows)) + for _, row := range t.Rows { + item := make(map[string]any, len(t.ColumnDefinitions)) + for ci, col := range t.ColumnDefinitions { + if ci < len(row.Cells) { + item[col.Name] = row.Cells[ci] + } + } + // Add namespace from the embedded object metadata if available + if row.Object.Object != nil { + if u, ok := row.Object.Object.(*unstructured.Unstructured); ok { + if ns := u.GetNamespace(); ns != "" { + item["Namespace"] = ns + } + } + } + result = append(result, item) + } + return result } func MarshalYaml(v any) (string, error) { diff --git a/pkg/output/output_test.go b/pkg/output/output_test.go index e77e40bc3..3a8ff94c3 100644 --- a/pkg/output/output_test.go +++ b/pkg/output/output_test.go @@ -5,29 +5,191 @@ import ( "regexp" "testing" + "github.com/stretchr/testify/suite" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" ) -func TestPlainTextUnstructuredList(t *testing.T) { +var podListJSON = `{ + "apiVersion": "v1", "kind": "PodList", "items": [{ + "apiVersion": "v1", "kind": "Pod", + "metadata": { + "name": "pod-1", "namespace": "default", "creationTimestamp": "2023-10-01T00:00:00Z", "labels": { "app": "nginx" } + }, + "spec": { "containers": [{ "name": "container-1", "image": "marcnuri/chuck-norris" }] } + }] +}` + +type OutputSuite struct { + suite.Suite +} + +func (s *OutputSuite) podList() *unstructured.UnstructuredList { var podList unstructured.UnstructuredList - _ = json.Unmarshal([]byte(` - { "apiVersion": "v1", "kind": "PodList", "items": [{ - "apiVersion": "v1", "kind": "Pod", - "metadata": { - "name": "pod-1", "namespace": "default", "creationTimestamp": "2023-10-01T00:00:00Z", "labels": { "app": "nginx" } - }, - "spec": { "containers": [{ "name": "container-1", "image": "marcnuri/chuck-norris" }] } } - ]}`), &podList) - out, err := Table.PrintObj(&podList) - t.Run("processes the list", func(t *testing.T) { - if err != nil { - t.Fatalf("Error printing pod list: %v", err) + s.Require().NoError(json.Unmarshal([]byte(podListJSON), &podList)) + return &podList +} + +func (s *OutputSuite) TestTablePrintObj() { + s.Run("processes the list", func() { + _, err := Table.PrintObj(s.podList()) + s.NoError(err) + }) + s.Run("prints headers", func() { + out, _ := Table.PrintObj(s.podList()) + m, err := regexp.MatchString("NAME\\s+AGE\\s+LABELS", out) + s.NoError(err) + s.True(m, "Expected headers not found in output: %s", out) + }) +} + +func (s *OutputSuite) TestTablePrintObjStructured() { + s.Run("returns non-nil result", func() { + result, err := Table.PrintObjStructured(s.podList()) + s.NoError(err) + s.NotNil(result) + }) + s.Run("text matches PrintObj output", func() { + text, _ := Table.PrintObj(s.podList()) + result, _ := Table.PrintObjStructured(s.podList()) + s.Equal(text, result.Text) + }) + s.Run("structured is nil for non-Table objects", func() { + result, err := Table.PrintObjStructured(s.podList()) + s.NoError(err) + // UnstructuredList is not a Table GVK, so structured will be nil + // unless the API returns a Table response + // The podList fixture is a PodList, not a Table + s.Nil(result.Structured) + }) +} + +func (s *OutputSuite) TestYamlPrintObjStructured() { + s.Run("returns non-nil result", func() { + result, err := Yaml.PrintObjStructured(s.podList()) + s.NoError(err) + s.NotNil(result) + }) + s.Run("text matches PrintObj output", func() { + text, _ := Yaml.PrintObj(s.podList()) + result, _ := Yaml.PrintObjStructured(s.podList()) + s.Equal(text, result.Text) + }) + s.Run("structured contains list items", func() { + result, err := Yaml.PrintObjStructured(s.podList()) + s.NoError(err) + items, ok := result.Structured.([]map[string]any) + s.Require().True(ok, "expected []map[string]any, got %T", result.Structured) + s.Len(items, 1) + }) + s.Run("structured items have expected fields", func() { + result, _ := Yaml.PrintObjStructured(s.podList()) + items := result.Structured.([]map[string]any) + item := items[0] + metadata, ok := item["metadata"].(map[string]any) + s.Require().True(ok) + s.Equal("pod-1", metadata["name"]) + s.Equal("default", metadata["namespace"]) + }) + s.Run("single object returns map", func() { + var pod unstructured.Unstructured + s.Require().NoError(json.Unmarshal([]byte(`{ + "apiVersion": "v1", "kind": "Pod", + "metadata": { "name": "single-pod", "namespace": "test" } + }`), &pod)) + result, err := Yaml.PrintObjStructured(&pod) + s.NoError(err) + obj, ok := result.Structured.(map[string]any) + s.Require().True(ok, "expected map[string]any for single object") + metadata, ok := obj["metadata"].(map[string]any) + s.Require().True(ok) + s.Equal("single-pod", metadata["name"]) + }) +} + +func (s *OutputSuite) TestTableToStructured() { + s.Run("returns nil for nil table", func() { + s.Nil(tableToStructured(nil)) + }) + s.Run("returns nil for table with no rows", func() { + t := &metav1.Table{ + ColumnDefinitions: []metav1.TableColumnDefinition{{Name: "Name"}}, + } + s.Nil(tableToStructured(t)) + }) + s.Run("extracts column-keyed maps from rows", func() { + t := &metav1.Table{ + ColumnDefinitions: []metav1.TableColumnDefinition{ + {Name: "Name"}, + {Name: "Status"}, + }, + Rows: []metav1.TableRow{ + {Cells: []any{"pod-1", "Running"}}, + {Cells: []any{"pod-2", "Pending"}}, + }, } + result := tableToStructured(t) + s.Require().Len(result, 2) + s.Equal("pod-1", result[0]["Name"]) + s.Equal("Running", result[0]["Status"]) + s.Equal("pod-2", result[1]["Name"]) + s.Equal("Pending", result[1]["Status"]) }) - t.Run("prints headers", func(t *testing.T) { - expectedHeaders := "NAME\\s+AGE\\s+LABELS" - if m, e := regexp.MatchString(expectedHeaders, out); !m || e != nil { - t.Errorf("Expected headers '%s' not found in output: %s", expectedHeaders, out) + s.Run("adds namespace from embedded object metadata", func() { + t := &metav1.Table{ + ColumnDefinitions: []metav1.TableColumnDefinition{ + {Name: "Name"}, + }, + Rows: []metav1.TableRow{ + { + Cells: []any{"pod-1"}, + Object: runtime.RawExtension{ + Object: &unstructured.Unstructured{ + Object: map[string]any{ + "metadata": map[string]any{ + "name": "pod-1", + "namespace": "kube-system", + }, + }, + }, + }, + }, + }, } + result := tableToStructured(t) + s.Require().Len(result, 1) + s.Equal("pod-1", result[0]["Name"]) + s.Equal("kube-system", result[0]["Namespace"]) }) + s.Run("does not add namespace when object has no namespace", func() { + t := &metav1.Table{ + ColumnDefinitions: []metav1.TableColumnDefinition{ + {Name: "Name"}, + }, + Rows: []metav1.TableRow{ + { + Cells: []any{"node-1"}, + Object: runtime.RawExtension{ + Object: &unstructured.Unstructured{ + Object: map[string]any{ + "metadata": map[string]any{ + "name": "node-1", + }, + }, + }, + }, + }, + }, + } + result := tableToStructured(t) + s.Require().Len(result, 1) + s.Equal("node-1", result[0]["Name"]) + _, hasNs := result[0]["Namespace"] + s.False(hasNs, "expected no Namespace key for cluster-scoped resource") + }) +} + +func TestOutput(t *testing.T) { + suite.Run(t, new(OutputSuite)) }