From e64f1688a803e36b0af615dc83fd27e3b98ca7aa Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Wed, 25 Mar 2026 05:44:02 +0100 Subject: [PATCH 1/3] refactor(output): add PrintObjStructured to Output interface Extend the Output interface with a PrintObjStructured method that returns both a human-readable text representation and optional structured data extracted from Kubernetes objects. This lays the groundwork for consumers that need programmatic access to the data alongside the formatted text. - Add PrintResult struct holding Text and Structured fields - Implement PrintObjStructured for yaml (returns deep-copied list items or object map) - Implement PrintObjStructured for table (extracts column-keyed maps via tableToStructured helper) - Refactor table.PrintObj to delegate to printTable for code reuse - Rewrite output tests to testify/suite pattern with new coverage Co-Authored-By: Claude Opus 4.6 Signed-off-by: Marc Nuri --- pkg/output/output.go | 82 +++++++++++++++- pkg/output/output_test.go | 196 ++++++++++++++++++++++++++++++++++---- 2 files changed, 260 insertions(+), 18 deletions(-) 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)) } From 80a1336db31aa28e149dff97ffeb276d0b47b594 Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Wed, 25 Mar 2026 05:45:45 +0100 Subject: [PATCH 2/3] feat(api): add NewToolCallResultFull constructor Add a NewToolCallResultFull constructor that creates a ToolCallResult with both human-readable text and structured content as separate fields. Unlike NewToolCallResultStructured (which JSON-serializes structured data into Content), this preserves the original text representation (e.g. table or YAML output) alongside the structured data. Refactor NewToolCallResult and NewToolCallResultStructured to delegate to NewToolCallResultFull, making it the single canonical constructor. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Marc Nuri --- pkg/api/toolsets.go | 19 ++++++++++++------- pkg/api/toolsets_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) 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"} From 5fe1ade835ea68ec3520bac06f273513b12788e0 Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Wed, 25 Mar 2026 05:47:38 +0100 Subject: [PATCH 3/3] fix(mcp): wrap slice structuredContent in object per MCP spec The MCP specification requires structuredContent to marshal to a JSON object, but NewStructuredResult could receive slice/array values. Add ensureStructuredObject to automatically wrap slices in {"items": [...]} so that the result always complies with the spec. Typed nil slices (e.g. []string(nil)) are detected via reflect and return nil to avoid producing {"items": null}. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Marc Nuri --- pkg/mcp/mcp.go | 26 +++++++++++++++++++++++++- pkg/mcp/text_result_test.go | 18 ++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) 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)