diff --git a/apps/backend/config/prompts/kandev-context.md b/apps/backend/config/prompts/kandev-context.md index ed3bb34ca0..3ea224cd33 100644 --- a/apps/backend/config/prompts/kandev-context.md +++ b/apps/backend/config/prompts/kandev-context.md @@ -17,6 +17,7 @@ Available tools: - get_task_plan_kandev: Read a task plan, including user edits. - update_task_plan_kandev: Update an existing task plan. - delete_task_plan_kandev: Delete a task plan. +- show_rich_output_kandev: When user asks for chart/graph/plot/file preview/KPI/metrics with data: call now. Do not implement the display as ASCII/SVG/HTML or with another app. Else prose; small text table: Markdown. Send version=1,title,blocks (1-4). Inline: {"type":"chart","chart_type":"bar","title":"T","summary":"S","labels":["A","B"],"series":[{"label":"Count","values":[42,27]}]}. CSV line: {"type":"chart","chart_type":"line","title":"T","summary":"S","csv":{"path":"reports/latency.csv","x_column":"recorded_at","series":[{"column":"p95_ms","label":"p95 (ms)"}]}}. Metrics: {"type":"metrics","items":[{"label":"Passed","value":"38"}]}. Paths workspace-relative. Kandev owns axes/legends/tooltips/layout. Label series with units. - show_walkthrough_kandev: Store an ordered, file-anchored code walkthrough. - get_walkthrough_kandev: Read a task's stored walkthrough. - delete_walkthrough_kandev: Delete a task's walkthrough. diff --git a/apps/backend/config/prompts/office-context.md b/apps/backend/config/prompts/office-context.md index 251af6d525..9b5a4fafde 100644 --- a/apps/backend/config/prompts/office-context.md +++ b/apps/backend/config/prompts/office-context.md @@ -11,6 +11,7 @@ Available tools: - get_task_plan_kandev: Retrieve the current task plan, including user edits. Required: task_id. - update_task_plan_kandev: Update the current task plan. Required: task_id, content. Optional: title. - delete_task_plan_kandev: Delete the current task plan. Required: task_id. +- show_rich_output_kandev: When user asks for chart/graph/plot/file preview/KPI/metrics with data: call now. Do not implement the display as ASCII/SVG/HTML or with another app. Else prose; small text table: Markdown. Send version=1,title,blocks (1-4). Inline: {"type":"chart","chart_type":"bar","title":"T","summary":"S","labels":["A","B"],"series":[{"label":"Count","values":[42,27]}]}. CSV line: {"type":"chart","chart_type":"line","title":"T","summary":"S","csv":{"path":"reports/latency.csv","x_column":"recorded_at","series":[{"column":"p95_ms","label":"p95 (ms)"}]}}. Metrics: {"type":"metrics","items":[{"label":"Passed","value":"38"}]}. Paths workspace-relative. Kandev owns axes/legends/tooltips/layout. Label series with units. - list_related_tasks_kandev: List parent, child, sibling, blocker, and blocked tasks. Optional: task_id (defaults to the current task), verbose (include task descriptions; omitted by default). - list_task_documents_kandev: List documents on an accessible related task. Required: task_id. - get_task_document_kandev: Read one document on an accessible related task. Required: task_id, document_key. diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_tools.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_tools.go index 193b0377e2..e7da6d5005 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_tools.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_tools.go @@ -127,7 +127,12 @@ func (a *Adapter) convertToolCallUpdate(sessionID string, tc *acp.SessionUpdateT } toolKind := string(tc.Kind) + toolName := toolKind normalizedPayload := a.normalizer.NormalizeToolCall(toolKind, args) + if frame, ok := a.dialect.parseMCPToolCall(tc.Meta, tc.RawInput); ok { + toolName = frame.name + normalizedPayload = streams.NewMCPTool(frame.name, frame.arguments) + } toolCallID := string(tc.ToolCallId) normalizedPayload, eventType, codexSignal, emittedToolCallID, codexParentToolCallID, suppress := a.trackToolCallPayload( @@ -147,7 +152,7 @@ func (a *Adapter) convertToolCallUpdate(sessionID string, tc *acp.SessionUpdateT a.handleWakeupEvent(sessionID, toolCallID, tc.Meta, tc.RawInput, false) // Detect tool type for logging - toolType := DetectToolOperationType(toolKind, args) + toolType := DetectToolOperationType(toolName, args) _ = toolType // Used for normalization status := string(tc.Status) @@ -168,7 +173,7 @@ func (a *Adapter) convertToolCallUpdate(sessionID string, tc *acp.SessionUpdateT SessionID: sessionID, ToolCallID: emittedToolCallID, ParentToolCallID: parentToolCallID, - ToolName: toolKind, // Kind is effectively the tool name + ToolName: toolName, ToolTitle: tc.Title, ToolStatus: status, NormalizedPayload: normalizedPayload, @@ -657,7 +662,8 @@ func (a *Adapter) convertToolCallResultUpdate(sessionID string, tcu *acp.Session return nil } monitorCommand := "" - if payload != nil && (tcu.RawInput != nil || len(tcu.Locations) > 0) { + canonicalMCPUpdate := payload != nil && payload.IsMCPTool() && payload.Generic() != nil + if payload != nil && !canonicalMCPUpdate && (tcu.RawInput != nil || len(tcu.Locations) > 0) { a.normalizer.UpdatePayloadInput(payload, tcu.RawInput, supplemental) } if payload != nil && (tcu.Title != nil || tcu.Meta != nil || tcu.RawInput != nil || supplemental != nil) { @@ -703,7 +709,13 @@ func (a *Adapter) convertToolCallResultUpdate(sessionID string, tcu *acp.Session if payload != nil && payload.Kind() == streams.ToolKindShellExec { recognizedShellUpdate = a.normalizer.NormalizeShellToolUpdate(payload, tcu.Meta, convertedContents, tcu.RawOutput) } else if tcu.RawOutput != nil && payload != nil && !isTrackedMonitorTerminal { - a.normalizer.NormalizeToolResult(payload, tcu.RawOutput) + result := tcu.RawOutput + if canonicalMCPUpdate { + if normalizedResult, ok := a.dialect.normalizeMCPToolResult(tcu.RawOutput); ok { + result = normalizedResult + } + } + a.normalizer.NormalizeToolResult(payload, result) } var shellExitCode *int if payload != nil && payload.ShellExec() != nil && payload.ShellExec().Output != nil { diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/codex_mcp_tool_test.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/codex_mcp_tool_test.go new file mode 100644 index 0000000000..24d74f0c4a --- /dev/null +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/codex_mcp_tool_test.go @@ -0,0 +1,136 @@ +package acp + +import ( + "testing" + + acpsdk "github.com/coder/acp-go-sdk" + "github.com/kandev/kandev/internal/agentctl/types/streams" + "github.com/stretchr/testify/require" +) + +func TestCodexMCPExecuteFrameNormalizesAsGenericToolCall(t *testing.T) { + t.Parallel() + + a := newTestAdapter() + a.agentID = codexAgentID + a.normalizer = NewNormalizer(codexAgentID) + a.dialect = newACPDialect(codexAgentID) + + arguments := map[string]any{ + "version": float64(1), + "title": "Failed requests", + "blocks": []any{map[string]any{ + "type": "chart", + "chart_type": "bar", + "title": "Failed requests by service", + "summary": "Failures in the latest interval.", + "labels": []any{"API", "Billing"}, + "series": []any{map[string]any{ + "label": "Failed requests", + "values": []any{float64(42), float64(27)}, + }}, + }}, + } + rawInput := map[string]any{ + "server": "kandev", + "tool": "show_rich_output_kandev", + "arguments": arguments, + } + + initial := a.convertToolCallUpdate("session-1", &acpsdk.SessionUpdateToolCall{ + Meta: map[string]any{"is_mcp_tool_call": true}, + Kind: acpsdk.ToolKind("execute"), + RawInput: rawInput, + SessionUpdate: "tool_call", + Status: acpsdk.ToolCallStatus("in_progress"), + Title: "mcp.kandev.show_rich_output_kandev", + ToolCallId: acpsdk.ToolCallId("mcp-call-1"), + }) + + require.NotNil(t, initial) + require.Equal(t, streams.EventTypeToolCall, initial.Type) + require.Equal(t, "kandev/show_rich_output_kandev", initial.ToolName) + require.Equal(t, streams.ToolKindGeneric, initial.NormalizedPayload.Kind()) + require.True(t, initial.NormalizedPayload.IsMCPTool()) + require.Equal(t, "kandev/show_rich_output_kandev", initial.NormalizedPayload.Generic().Name) + require.Equal(t, arguments, initial.NormalizedPayload.Generic().Input) + require.Nil(t, initial.NormalizedPayload.ShellExec()) + + structured := map[string]any{ + "version": float64(1), + "resolved_charts": []any{map[string]any{ + "block_index": float64(0), + "labels": []any{"API", "Billing"}, + "series": []any{map[string]any{ + "label": "Failed requests", + "values": []any{float64(42), float64(27)}, + }}, + }}, + } + callToolResult := map[string]any{ + "content": []any{map[string]any{ + "type": "text", + "text": `{"version":1,"resolved_charts":[]}`, + }}, + "structuredContent": structured, + } + completed := acpsdk.ToolCallStatus("completed") + terminal := a.convertToolCallResultUpdate("session-1", &acpsdk.SessionToolCallUpdate{ + RawInput: rawInput, + RawOutput: map[string]any{"error": nil, "result": callToolResult}, + SessionUpdate: "tool_call_update", + Status: &completed, + ToolCallId: acpsdk.ToolCallId("mcp-call-1"), + }) + + require.NotNil(t, terminal) + require.Equal(t, streams.EventTypeToolUpdate, terminal.Type) + require.Equal(t, toolStatusComplete, terminal.ToolStatus) + require.Equal(t, streams.ToolKindGeneric, terminal.NormalizedPayload.Kind()) + require.True(t, terminal.NormalizedPayload.IsMCPTool()) + require.Equal(t, arguments, terminal.NormalizedPayload.Generic().Input) + require.Equal(t, callToolResult, terminal.NormalizedPayload.Generic().Output) +} + +func TestCodexMCPExecuteFramePreservesServerIdentity(t *testing.T) { + t.Parallel() + + a := newTestAdapter() + a.agentID = codexAgentID + a.normalizer = NewNormalizer(codexAgentID) + a.dialect = newACPDialect(codexAgentID) + + event := a.convertToolCallUpdate("session-1", &acpsdk.SessionUpdateToolCall{ + Meta: map[string]any{"is_mcp_tool_call": true}, + Kind: acpsdk.ToolKind("execute"), + RawInput: map[string]any{"server": "other", "tool": "show_rich_output_kandev", "arguments": map[string]any{}}, + SessionUpdate: "tool_call", + Status: acpsdk.ToolCallStatus("in_progress"), + ToolCallId: acpsdk.ToolCallId("mcp-call-foreign-server"), + }) + + require.NotNil(t, event) + require.Equal(t, "other/show_rich_output_kandev", event.NormalizedPayload.Generic().Name) +} + +func TestCodexExecuteWithoutMCPMarkerRemainsShellExecution(t *testing.T) { + t.Parallel() + + a := newTestAdapter() + a.agentID = codexAgentID + a.normalizer = NewNormalizer(codexAgentID) + a.dialect = newACPDialect(codexAgentID) + + event := a.convertToolCallUpdate("session-1", &acpsdk.SessionUpdateToolCall{ + Kind: acpsdk.ToolKind("execute"), + RawInput: map[string]any{"command": "pwd"}, + SessionUpdate: "tool_call", + Status: acpsdk.ToolCallStatus("in_progress"), + Title: "Run pwd", + ToolCallId: acpsdk.ToolCallId("shell-call-1"), + }) + + require.NotNil(t, event) + require.Equal(t, streams.ToolKindShellExec, event.NormalizedPayload.Kind()) + require.Equal(t, "pwd", event.NormalizedPayload.ShellExec().Command) +} diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go index e1acea0ba6..7a68af2292 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go @@ -25,6 +25,13 @@ type acpDialect struct { contextWindow func(map[string]any, []modelInfo, []streams.ConfigOption) (contextWindowSample, bool) normalizePromptUsage func(*streams.PromptUsage, map[string]any) *streams.PromptUsage subagentFrame func(map[string]any, string, any) (subagentFrame, bool) + mcpToolCall func(map[string]any, any) (mcpToolCallFrame, bool) + mcpToolResult func(any) (any, bool) +} + +type mcpToolCallFrame struct { + name string + arguments map[string]any } type dialectConfigChange struct { @@ -62,6 +69,20 @@ func (d acpDialect) parseSubagentFrame(meta map[string]any, title string, rawInp return d.subagentFrame(meta, title, rawInput) } +func (d acpDialect) parseMCPToolCall(meta map[string]any, rawInput any) (mcpToolCallFrame, bool) { + if d.mcpToolCall == nil { + return mcpToolCallFrame{}, false + } + return d.mcpToolCall(meta, rawInput) +} + +func (d acpDialect) normalizeMCPToolResult(rawOutput any) (any, bool) { + if d.mcpToolResult == nil { + return nil, false + } + return d.mcpToolResult(rawOutput) +} + func (d acpDialect) sessionConfigOptions( meta map[string]any, options []acp.SessionConfigOption, diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_codex.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_codex.go index 221c6165e5..3451e8a97a 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_codex.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_codex.go @@ -31,6 +31,8 @@ func newCodexACPDialect() acpDialect { return acpDialect{ subagentFrame: parseCodexSubagentFrame, normalizePromptUsage: normalizeCodexPromptUsage, + mcpToolCall: parseCodexMCPToolCall, + mcpToolResult: normalizeCodexMCPToolResult, } } @@ -57,6 +59,44 @@ func normalizeCodexPromptUsage( return usage } +// parseCodexMCPToolCall recognizes Codex's observed MCP-over-ACP envelope. +// Codex reports these as the broad ACP "execute" kind and puts the actual MCP +// identity and arguments in rawInput. Require the explicit implementation +// marker plus the complete envelope so ordinary execute tools remain shells. +func parseCodexMCPToolCall(meta map[string]any, rawInput any) (mcpToolCallFrame, bool) { + isMCP, _ := meta["is_mcp_tool_call"].(bool) + if !isMCP { + return mcpToolCallFrame{}, false + } + input, ok := rawInput.(map[string]any) + if !ok { + return mcpToolCallFrame{}, false + } + server, _ := input["server"].(string) + tool, _ := input["tool"].(string) + arguments, ok := input["arguments"].(map[string]any) + server = strings.TrimSpace(server) + tool = strings.TrimSpace(tool) + if server == "" || tool == "" || !ok { + return mcpToolCallFrame{}, false + } + return mcpToolCallFrame{name: server + "/" + tool, arguments: arguments}, true +} + +// normalizeCodexMCPToolResult removes Codex's transport-only +// {error, result} wrapper while retaining the standard MCP CallToolResult. +func normalizeCodexMCPToolResult(rawOutput any) (any, bool) { + output, ok := rawOutput.(map[string]any) + if !ok { + return rawOutput, rawOutput != nil + } + result, exists := output["result"] + if !exists { + return rawOutput, true + } + return result, true +} + // codexSystemErrorMeta reports the explicit thread-status marker emitted by // codex-acp when a prompt failed at the provider boundary. The marker is // implementation metadata, so keep its shape local to the Codex dialect and diff --git a/apps/backend/internal/agentctl/types/streams/tool_payload.go b/apps/backend/internal/agentctl/types/streams/tool_payload.go index e8bd9c4de9..e5de39ec9f 100644 --- a/apps/backend/internal/agentctl/types/streams/tool_payload.go +++ b/apps/backend/internal/agentctl/types/streams/tool_payload.go @@ -69,6 +69,10 @@ type NormalizedPayload struct { // gates on ACP `_meta.claudeCode.toolName` — set by the claude-agent-acp // wrapper, not by model tool output. IsActiveMonitor classifies on this. monitor *MonitorPayload + // mcpTool is trusted adapter provenance for a recognized MCP execution. + // It stays internal because persistence should contain the canonical tool + // name, input, and result rather than transport-specific metadata. + mcpTool bool } // --- Getters for NormalizedPayload --- @@ -85,6 +89,7 @@ func (p *NormalizedPayload) SubagentTask() *SubagentTaskPayload { return p.subag func (p *NormalizedPayload) ShowPlan() *ShowPlanPayload { return p.showPlan } func (p *NormalizedPayload) ManageTodos() *ManageTodosPayload { return p.manageTodos } func (p *NormalizedPayload) Misc() *MiscPayload { return p.misc } +func (p *NormalizedPayload) IsMCPTool() bool { return p != nil && p.mcpTool } // Snapshot returns a detached copy of the normalized payload. A snapshot owns // every pointer, slice, and JSON-compatible value reachable from the payload so @@ -108,6 +113,7 @@ func (p *NormalizedPayload) Snapshot() *NormalizedPayload { misc: cloneMiscPayload(p.misc), backgroundWork: cloneBackgroundWorkPayload(p.backgroundWork), monitor: cloneMonitorPayload(p.monitor), + mcpTool: p.mcpTool, } } @@ -616,6 +622,13 @@ func NewGeneric(name string, input any) *NormalizedPayload { } } +// NewMCPTool creates a generic payload with trusted MCP transport provenance. +func NewMCPTool(name string, input any) *NormalizedPayload { + payload := NewGeneric(name, input) + payload.mcpTool = true + return payload +} + // NewSubagentTask creates a NormalizedPayload for subagent (Task) tool calls. // Result fields (status, agent id, metrics, …) are filled later from the // completion tool_call_update via the ACP normalizer's EnrichSubagentResult. diff --git a/apps/backend/internal/mcp/server/rich_output.go b/apps/backend/internal/mcp/server/rich_output.go new file mode 100644 index 0000000000..d32e8a975a --- /dev/null +++ b/apps/backend/internal/mcp/server/rich_output.go @@ -0,0 +1,282 @@ +package mcp + +import ( + "context" + "encoding/json" + "fmt" + "path" + "strings" + + mcplib "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +const ( + richOutputToolName = "show_rich_output_kandev" + richOutputBlockTypeChart = "chart" + richOutputBlockTypeFile = "file" + richOutputFieldPath = "path" + richOutputFieldSessionID = "session_id" + maxRichOutputPayloadBytes = 64 * 1024 +) + +var richOutputInputSchema = json.RawMessage(`{ + "type": "object", + "description": "One compact Kandev-native presentation. Use semantic data only; Kandev owns layout and styling.", + "properties": { + "version": {"type": "integer", "const": 1, "description": "Rich-output contract version. Must be 1."}, + "title": {"type": "string", "minLength": 1, "maxLength": 120, "description": "Plain-text presentation title."}, + "description": {"type": "string", "maxLength": 500, "description": "Optional plain-text context for the full presentation."}, + "blocks": { + "type": "array", + "minItems": 1, + "maxItems": 4, + "description": "One to four ordered file, chart, or metrics blocks.", + "items": { + "oneOf": [ + { + "type": "object", + "properties": { + "type": {"const": "file", "description": "Workspace file preview block."}, + "path": {"type": "string", "minLength": 1, "maxLength": 1024, "description": "Workspace-relative file path."}, + "repo": {"type": "string", "maxLength": 255, "description": "Optional repository discriminator for multi-repository tasks."}, + "title": {"type": "string", "maxLength": 120, "description": "Optional plain-text file title."}, + "caption": {"type": "string", "maxLength": 500, "description": "Optional plain-text reason to inspect the file."}, + "mime_type": {"type": "string", "maxLength": 128, "description": "Optional media type hint."} + }, + "required": ["type", "path"], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "type": {"const": "chart", "description": "Native line or bar chart with host-owned axes, tooltip, and legend."}, + "chart_type": {"type": "string", "enum": ["line", "bar"], "description": "Use line for trends or time series; bar for category comparisons."}, + "title": {"type": "string", "minLength": 1, "maxLength": 120, "description": "Plain-text chart title."}, + "summary": {"type": "string", "minLength": 1, "maxLength": 500, "description": "Plain-text nonvisual interpretation of the chart."}, + "labels": { + "type": "array", + "minItems": 1, + "maxItems": 100, + "description": "Inline x-axis labels. Use with inline series, not csv.", + "items": {"type": "string", "minLength": 1, "maxLength": 120} + }, + "series": { + "type": "array", + "minItems": 1, + "maxItems": 4, + "description": "Inline numeric series. Each values array must match labels.", + "items": { + "type": "object", + "properties": { + "label": {"type": "string", "minLength": 1, "maxLength": 120, "description": "Legend label; include the unit when useful."}, + "values": { + "type": "array", + "minItems": 1, + "maxItems": 100, + "items": {"type": ["number", "null"]} + } + }, + "required": ["label", "values"], + "additionalProperties": false + } + }, + "csv": { + "type": "object", + "description": "Load bounded chart data from one workspace-relative CSV instead of inlining rows.", + "properties": { + "path": {"type": "string", "minLength": 1, "maxLength": 1024, "description": "Workspace-relative CSV path ending in .csv."}, + "repo": {"type": "string", "maxLength": 255, "description": "Optional repository discriminator for a multi-repository task."}, + "x_column": {"type": "string", "minLength": 1, "maxLength": 120, "description": "Exact CSV header used for x-axis category or time labels. Rows render in file order."}, + "series": { + "type": "array", + "minItems": 1, + "maxItems": 4, + "description": "One to four CSV numeric column mappings.", + "items": { + "type": "object", + "properties": { + "column": {"type": "string", "minLength": 1, "maxLength": 120, "description": "Exact CSV header for a numeric column."}, + "label": {"type": "string", "minLength": 1, "maxLength": 120, "description": "Optional legend label; include the unit when useful. Defaults to column."} + }, + "required": ["column"], + "additionalProperties": false + } + } + }, + "required": ["path", "x_column", "series"], + "additionalProperties": false + } + }, + "required": ["type", "chart_type", "title", "summary"], + "oneOf": [ + { + "required": ["labels", "series"], + "not": {"required": ["csv"]} + }, + { + "required": ["csv"], + "not": { + "anyOf": [ + {"required": ["labels"]}, + {"required": ["series"]} + ] + } + } + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "type": {"const": "metrics", "description": "Compact metric group."}, + "items": { + "type": "array", + "minItems": 1, + "maxItems": 6, + "items": { + "type": "object", + "properties": { + "label": {"type": "string", "minLength": 1, "maxLength": 120}, + "value": {"type": "string", "minLength": 1, "maxLength": 200}, + "detail": {"type": "string", "maxLength": 500} + }, + "required": ["label", "value"], + "additionalProperties": false + } + } + }, + "required": ["type", "items"], + "additionalProperties": false + } + ] + } + } + }, + "examples": [ + {"version":1,"title":"Service failures","blocks":[{"type":"chart","chart_type":"bar","title":"Failures by service","summary":"Failed requests in the latest interval.","labels":["A","B"],"series":[{"label":"Count","values":[42,27]}]}]}, + {"version":1,"title":"Latency trend","blocks":[{"type":"chart","chart_type":"line","title":"p95 latency","summary":"Latency across recorded samples.","csv":{"path":"reports/latency.csv","x_column":"recorded_at","series":[{"column":"p95_ms","label":"p95 (ms)"}]}}]}, + {"version":1,"title":"Requests by route","blocks":[{"type":"chart","chart_type":"bar","title":"Route volume","summary":"Request count for each route.","csv":{"path":"reports/routes.csv","x_column":"route","series":[{"column":"requests"}]}}]} + ], + "required": ["version", "title", "blocks"], + "additionalProperties": false +}`) + +func (s *Server) registerRichOutputTool() { + tool := mcplib.NewToolWithRawSchema( + richOutputToolName, + `Present one native file preview, metric group, line chart, bar chart, or CSV chart. For an explicit chart, graph, plot, preview, KPI, or metrics request with data, call directly. Do not implement the display as ASCII, SVG, HTML, or with another app. Use Markdown only for small text tables and prose otherwise. Supply semantic data; Kandev owns axes, legends, tooltips, layout, and styling. CSV paths are task-workspace-relative. Label series with units.`, + richOutputInputSchema, + ) + tool.Annotations.ReadOnlyHint = mcplib.ToBoolPtr(true) + tool.Annotations.DestructiveHint = mcplib.ToBoolPtr(false) + s.mcpServer.AddTool(tool, s.wrapHandler(richOutputToolName, s.showRichOutputHandler())) +} + +func (s *Server) showRichOutputHandler() server.ToolHandlerFunc { + return func(ctx context.Context, req mcplib.CallToolRequest) (*mcplib.CallToolResult, error) { + if err := validateRichOutput(req.GetArguments()); err != nil { + return mcplib.NewToolResultError(err.Error()), nil + } + snapshot, err := s.resolveRichOutputCSV(ctx, req.GetArguments()) + if err != nil { + return mcplib.NewToolResultError(err.Error()), nil + } + if snapshot != nil { + return mcplib.NewToolResultStructuredOnly(snapshot), nil + } + return mcplib.NewToolResultText("Rich output accepted."), nil + } +} + +func validateRichOutput(args map[string]interface{}) error { + encoded, err := json.Marshal(args) + if err != nil { + return fmt.Errorf("rich output payload is invalid") + } + if len(encoded) > maxRichOutputPayloadBytes { + return fmt.Errorf("rich output exceeds 64 KiB") + } + blocks, ok := args["blocks"].([]interface{}) + if !ok { + return fmt.Errorf("rich output blocks are invalid") + } + for _, rawBlock := range blocks { + block, ok := rawBlock.(map[string]interface{}) + if !ok { + continue + } + switch block["type"] { + case richOutputBlockTypeChart: + if err := validateRichOutputChart(block); err != nil { + return err + } + case richOutputBlockTypeFile: + filePath, _ := block[richOutputFieldPath].(string) + if !isWorkspaceRelativeRichOutputPath(filePath) { + return fmt.Errorf("file path must be workspace relative") + } + } + } + return nil +} + +func isWorkspaceRelativeRichOutputPath(value string) bool { + if strings.TrimSpace(value) == "" || strings.ContainsRune(value, '\x00') { + return false + } + normalized := strings.ReplaceAll(value, `\`, "/") + lower := strings.ToLower(normalized) + if strings.HasPrefix(normalized, "/") || strings.Contains(lower, "://") || strings.HasPrefix(lower, "data:") { + return false + } + if len(normalized) >= 2 && isASCIILetter(normalized[0]) && normalized[1] == ':' { + return false + } + for _, segment := range strings.Split(normalized, "/") { + if segment == ".." { + return false + } + } + return path.Clean(normalized) != "." +} + +func isASCIILetter(value byte) bool { + return value >= 'a' && value <= 'z' || value >= 'A' && value <= 'Z' +} + +func validateRichOutputChart(block map[string]interface{}) error { + if rawCSV, exists := block["csv"]; exists { + csvSource, ok := rawCSV.(map[string]interface{}) + if !ok { + return fmt.Errorf("chart CSV source is invalid") + } + filePath, _ := csvSource[richOutputFieldPath].(string) + if !isWorkspaceRelativeRichOutputPath(filePath) { + return fmt.Errorf("CSV path must be workspace relative") + } + if !strings.EqualFold(path.Ext(filePath), ".csv") { + return fmt.Errorf("CSV path must end in .csv") + } + return nil + } + labels, ok := block["labels"].([]interface{}) + if !ok { + return fmt.Errorf("chart labels are invalid") + } + series, ok := block["series"].([]interface{}) + if !ok { + return fmt.Errorf("chart series are invalid") + } + for _, rawSeries := range series { + item, ok := rawSeries.(map[string]interface{}) + if !ok { + return fmt.Errorf("chart series are invalid") + } + values, ok := item["values"].([]interface{}) + if !ok || len(values) != len(labels) { + return fmt.Errorf("chart series values must match labels") + } + } + return nil +} diff --git a/apps/backend/internal/mcp/server/rich_output_csv.go b/apps/backend/internal/mcp/server/rich_output_csv.go new file mode 100644 index 0000000000..02f19d1c8e --- /dev/null +++ b/apps/backend/internal/mcp/server/rich_output_csv.go @@ -0,0 +1,269 @@ +package mcp + +import ( + "context" + "encoding/csv" + "encoding/json" + "errors" + "fmt" + "io" + "math" + "strconv" + "strings" + "unicode/utf8" + + "github.com/kandev/kandev/internal/agentctl/types/streams" + ws "github.com/kandev/kandev/pkg/websocket" +) + +const ( + maxRichOutputCSVBytes = 256 * 1024 + maxRichOutputCSVRows = 100 + maxRichOutputChartText = 120 +) + +type richOutputCSVSeriesInput struct { + Column string `json:"column"` + Label string `json:"label,omitempty"` +} + +type richOutputCSVSourceInput struct { + Path string `json:"path"` + Repo string `json:"repo,omitempty"` + XColumn string `json:"x_column"` + Series []richOutputCSVSeriesInput `json:"series"` +} + +type richOutputCSVBlockInput struct { + CSV *richOutputCSVSourceInput `json:"csv,omitempty"` +} + +type richOutputCSVPresentationInput struct { + Blocks []richOutputCSVBlockInput `json:"blocks"` +} + +type richOutputResolvedSeries struct { + Label string `json:"label"` + Values []*float64 `json:"values"` +} + +type richOutputResolvedChart struct { + BlockIndex int `json:"block_index"` + Labels []string `json:"labels"` + Series []richOutputResolvedSeries `json:"series"` +} + +type richOutputCSVSnapshot struct { + Version int `json:"version"` + ResolvedCharts []richOutputResolvedChart `json:"resolved_charts"` +} + +func (s *Server) resolveRichOutputCSV( + ctx context.Context, + args map[string]interface{}, +) (*richOutputCSVSnapshot, error) { + presentation, err := decodeRichOutputCSVInput(args) + if err != nil { + return nil, err + } + snapshot := &richOutputCSVSnapshot{Version: 1} + cache := make(map[string]*streams.FileContentResponse) + for blockIndex, block := range presentation.Blocks { + if block.CSV == nil { + continue + } + response, readErr := s.readRichOutputCSV(ctx, block.CSV, cache) + if readErr != nil { + return nil, readErr + } + resolved, parseErr := parseRichOutputCSV(*block.CSV, response.Content, blockIndex) + if parseErr != nil { + return nil, fmt.Errorf("CSV source %s: %w", block.CSV.Path, parseErr) + } + snapshot.ResolvedCharts = append(snapshot.ResolvedCharts, resolved) + } + if len(snapshot.ResolvedCharts) == 0 { + return nil, nil + } + encoded, err := json.Marshal(snapshot) + if err != nil || len(encoded) > maxRichOutputPayloadBytes { + return nil, fmt.Errorf("normalized CSV chart snapshot exceeds 64 KiB") + } + return snapshot, nil +} + +func decodeRichOutputCSVInput(args map[string]interface{}) (richOutputCSVPresentationInput, error) { + encoded, err := json.Marshal(args) + if err != nil { + return richOutputCSVPresentationInput{}, fmt.Errorf("rich output payload is invalid") + } + var presentation richOutputCSVPresentationInput + if err := json.Unmarshal(encoded, &presentation); err != nil { + return richOutputCSVPresentationInput{}, fmt.Errorf("rich output payload is invalid") + } + return presentation, nil +} + +func (s *Server) readRichOutputCSV( + ctx context.Context, + source *richOutputCSVSourceInput, + cache map[string]*streams.FileContentResponse, +) (*streams.FileContentResponse, error) { + cacheKey := source.Repo + "\x00" + source.Path + if cached := cache[cacheKey]; cached != nil { + return cached, nil + } + payload := map[string]interface{}{richOutputFieldSessionID: s.sessionID, richOutputFieldPath: source.Path} + if source.Repo != "" { + payload["repo"] = source.Repo + } + var response streams.FileContentResponse + if err := s.backend.RequestPayload(ctx, ws.ActionWorkspaceFileContentGet, payload, &response); err != nil { + return nil, fmt.Errorf("could not read CSV source %s: %w", source.Path, err) + } + if response.Error != "" { + return nil, fmt.Errorf("could not read CSV source %s: %s", source.Path, response.Error) + } + if response.IsBinary || !utf8.ValidString(response.Content) { + return nil, fmt.Errorf("CSV source must be UTF-8 text") + } + if response.Size > maxRichOutputCSVBytes || len(response.Content) > maxRichOutputCSVBytes { + return nil, fmt.Errorf("CSV source exceeds 256 KiB") + } + cache[cacheKey] = &response + return &response, nil +} + +func parseRichOutputCSV( + source richOutputCSVSourceInput, + content string, + blockIndex int, +) (richOutputResolvedChart, error) { + reader := csv.NewReader(strings.NewReader(content)) + header, err := reader.Read() + if errors.Is(err, io.EOF) { + return richOutputResolvedChart{}, fmt.Errorf("CSV source must contain a header row") + } + if err != nil { + return richOutputResolvedChart{}, fmt.Errorf("CSV header is invalid: %w", err) + } + headerIndexes, err := indexRichOutputCSVHeaders(header) + if err != nil { + return richOutputResolvedChart{}, err + } + xIndex, seriesIndexes, err := resolveRichOutputCSVColumns(source, headerIndexes) + if err != nil { + return richOutputResolvedChart{}, err + } + reader.FieldsPerRecord = len(header) + resolved := newRichOutputResolvedChart(source, blockIndex) + for rowNumber := 2; ; rowNumber++ { + record, readErr := reader.Read() + if errors.Is(readErr, io.EOF) { + break + } + if readErr != nil { + return richOutputResolvedChart{}, fmt.Errorf("CSV row %d is invalid: %w", rowNumber, readErr) + } + if len(resolved.Labels) == maxRichOutputCSVRows { + return richOutputResolvedChart{}, fmt.Errorf("CSV source must contain at most 100 data rows") + } + if err := appendRichOutputCSVRow(&resolved, source, record, rowNumber, xIndex, seriesIndexes); err != nil { + return richOutputResolvedChart{}, err + } + } + if len(resolved.Labels) == 0 { + return richOutputResolvedChart{}, fmt.Errorf("CSV source must contain 1 to 100 data rows") + } + return resolved, nil +} + +func indexRichOutputCSVHeaders(header []string) (map[string]int, error) { + indexes := make(map[string]int, len(header)) + for index, rawName := range header { + name := strings.TrimSpace(strings.TrimPrefix(rawName, "\ufeff")) + if name == "" { + return nil, fmt.Errorf("CSV header %d must not be blank", index+1) + } + if _, exists := indexes[name]; exists { + return nil, fmt.Errorf("CSV header %q is duplicated", name) + } + indexes[name] = index + } + return indexes, nil +} + +func resolveRichOutputCSVColumns( + source richOutputCSVSourceInput, + headers map[string]int, +) (int, []int, error) { + xIndex, ok := headers[source.XColumn] + if !ok { + return 0, nil, fmt.Errorf("CSV column %q was not found", source.XColumn) + } + seriesIndexes := make([]int, len(source.Series)) + for index, series := range source.Series { + columnIndex, exists := headers[series.Column] + if !exists { + return 0, nil, fmt.Errorf("CSV column %q was not found", series.Column) + } + seriesIndexes[index] = columnIndex + } + return xIndex, seriesIndexes, nil +} + +func newRichOutputResolvedChart( + source richOutputCSVSourceInput, + blockIndex int, +) richOutputResolvedChart { + resolved := richOutputResolvedChart{BlockIndex: blockIndex} + for _, input := range source.Series { + label := input.Label + if label == "" { + label = input.Column + } + resolved.Series = append(resolved.Series, richOutputResolvedSeries{Label: label}) + } + return resolved +} + +func appendRichOutputCSVRow( + resolved *richOutputResolvedChart, + source richOutputCSVSourceInput, + record []string, + rowNumber, xIndex int, + seriesIndexes []int, +) error { + label := strings.TrimSpace(record[xIndex]) + if label == "" { + return fmt.Errorf("CSV row %d column %q must not be blank", rowNumber, source.XColumn) + } + if utf8.RuneCountInString(label) > maxRichOutputChartText { + return fmt.Errorf("CSV row %d column %q exceeds 120 characters", rowNumber, source.XColumn) + } + resolved.Labels = append(resolved.Labels, label) + for index, columnIndex := range seriesIndexes { + value, err := parseRichOutputCSVNumber(record[columnIndex]) + if err != nil { + return fmt.Errorf( + "CSV row %d column %q must be a finite number or empty", + rowNumber, + source.Series[index].Column, + ) + } + resolved.Series[index].Values = append(resolved.Series[index].Values, value) + } + return nil +} + +func parseRichOutputCSVNumber(raw string) (*float64, error) { + value := strings.TrimSpace(raw) + if value == "" { + return nil, nil + } + number, err := strconv.ParseFloat(value, 64) + if err != nil || math.IsNaN(number) || math.IsInf(number, 0) { + return nil, fmt.Errorf("not finite") + } + return &number, nil +} diff --git a/apps/backend/internal/mcp/server/rich_output_test.go b/apps/backend/internal/mcp/server/rich_output_test.go new file mode 100644 index 0000000000..60518d1fbb --- /dev/null +++ b/apps/backend/internal/mcp/server/rich_output_test.go @@ -0,0 +1,441 @@ +package mcp + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + ws "github.com/kandev/kandev/pkg/websocket" + "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRichOutputToolRegistration(t *testing.T) { + tests := []struct { + name string + mode string + want bool + }{ + {name: "task", mode: ModeTask, want: true}, + {name: "office", mode: ModeOffice, want: true}, + {name: "configuration", mode: ModeConfig, want: false}, + {name: "external", mode: ModeExternal, want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := New(&testBackend{}, "session-1", "task-1", 10005, newTestLogger(t), "", false, tt.mode) + _, got := s.mcpServer.ListTools()["show_rich_output_kandev"] + assert.Equal(t, tt.want, got) + }) + } +} + +func TestRichOutputToolAcceptsValidPayload(t *testing.T) { + s := New(&testBackend{}, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + + result := callTool(t, s, richOutputToolName, validRichOutputArgs()) + + require.False(t, result.IsError) + require.Len(t, result.Content, 1) +} + +func TestRichOutputToolResolvesCSVChartSnapshot(t *testing.T) { + csv := "recorded_at,p50_ms,p95_ms\n2026-08-13T10:00:00Z,18.2,29.4\n2026-08-14T10:00:00Z,16.4,\n" + backend := &testBackend{response: map[string]interface{}{ + "path": "reports/latency.csv", "content": csv, "size": len(csv), "is_binary": false, + }} + s := New(backend, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + + result := callTool(t, s, richOutputToolName, csvRichOutputArgs()) + + require.False(t, result.IsError) + assert.Equal(t, ws.ActionWorkspaceFileContentGet, backend.lastAction) + assert.Equal(t, map[string]interface{}{ + "session_id": "session-1", "path": "reports/latency.csv", "repo": "api", + }, backend.lastPayload) + want := map[string]interface{}{ + "version": 1, + "resolved_charts": []interface{}{ + map[string]interface{}{ + "block_index": 0, + "labels": []interface{}{"2026-08-13T10:00:00Z", "2026-08-14T10:00:00Z"}, + "series": []interface{}{ + map[string]interface{}{"label": "p50", "values": []interface{}{18.2, 16.4}}, + map[string]interface{}{"label": "p95_ms", "values": []interface{}{29.4, nil}}, + }, + }, + }, + } + assertJSONEquivalent(t, want, result.StructuredContent) + require.Len(t, result.Content, 1) + textContent, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + var fallback map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &fallback)) + assertJSONEquivalent(t, want, fallback) +} + +func TestRichOutputToolRejectsInvalidCSVData(t *testing.T) { + tooManyRows := "x,value\n" + for i := 0; i < 101; i++ { + tooManyRows += fmt.Sprintf("%d,%d\n", i, i) + } + tooLongLabel := strings.Repeat("x", 121) + tests := []struct { + name string + content string + size int + binary bool + mutate func(map[string]interface{}) + want string + }{ + {name: "binary", content: "AAEC", size: 3, binary: true, want: "CSV source must be UTF-8 text"}, + {name: "oversized", content: "x,value\na,1\n", size: 256*1024 + 1, want: "CSV source exceeds 256 KiB"}, + {name: "header only", content: "x,value\n", size: 8, want: "CSV source must contain 1 to 100 data rows"}, + {name: "too many rows", content: tooManyRows, size: len(tooManyRows), want: "CSV source must contain at most 100 data rows"}, + {name: "duplicate header", content: "x,value,value\na,1,2\n", size: 20, want: `CSV header "value" is duplicated`}, + {name: "missing x column", content: "when,value\na,1\n", size: 15, want: `CSV column "x" was not found`}, + {name: "missing series column", content: "x,other\na,1\n", size: 12, want: `CSV column "value" was not found`}, + {name: "blank x value", content: "x,value\n,1\n", size: 11, want: `CSV row 2 column "x" must not be blank`}, + {name: "long x value", content: "x,value\n" + tooLongLabel + ",1\n", size: 132, want: `CSV row 2 column "x" exceeds 120 characters`}, + {name: "bad numeric value", content: "x,value\na,fast\n", size: 15, want: `CSV row 2 column "value" must be a finite number or empty`}, + {name: "uneven row", content: "x,value\na,1,extra\n", size: 18, want: "CSV row 2"}, + { + name: "unsafe path", content: "x,value\na,1\n", size: 12, want: "CSV path must be workspace relative", + mutate: func(args map[string]interface{}) { + csvChartSource(args)["path"] = "../report.csv" + }, + }, + { + name: "non CSV path", content: "x,value\na,1\n", size: 12, want: "CSV path must end in .csv", + mutate: func(args map[string]interface{}) { + csvChartSource(args)["path"] = "reports/report.txt" + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + backend := &testBackend{response: map[string]interface{}{ + "path": "reports/latency.csv", "content": tt.content, "size": tt.size, "is_binary": tt.binary, + }} + s := New(backend, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + args := csvRichOutputArgs() + source := csvChartSource(args) + source["x_column"] = "x" + source["series"] = []interface{}{map[string]interface{}{"column": "value"}} + if tt.mutate != nil { + tt.mutate(args) + } + + result := callTool(t, s, richOutputToolName, args) + + require.True(t, result.IsError) + content, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Contains(t, content.Text, tt.want) + }) + } +} + +func TestRichOutputToolReportsCSVReadFailure(t *testing.T) { + backend := &testBackend{err: fmt.Errorf("workspace offline")} + s := New(backend, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + + result := callTool(t, s, richOutputToolName, csvRichOutputArgs()) + + require.True(t, result.IsError) + content, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Contains(t, content.Text, "could not read CSV source reports/latency.csv") +} + +func TestRichOutputToolDescriptionStaysFocusedAndDiscoverable(t *testing.T) { + s := New(&testBackend{}, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + tool := s.mcpServer.ListTools()[richOutputToolName].Tool + + for _, phrase := range []string{"file preview", "line chart", "bar chart", "CSV chart", "graph", "plot", "KPI", "metrics"} { + assert.Contains(t, tool.Description, phrase) + } + for _, phrase := range []string{ + "explicit chart, graph, plot, preview, KPI, or metrics request with data, call directly", + "Do not implement the display as ASCII, SVG, HTML, or with another app", + "Kandev owns axes, legends, tooltips, layout, and styling", + "task-workspace-relative", + "Markdown only for small text tables", + "Label series with units", + } { + assert.Contains(t, tool.Description, phrase) + } + assert.NotContains(t, tool.Description, "Inline chart recipe") + assert.NotContains(t, tool.Description, "CSV chart recipe") + assert.NotContains(t, tool.Description, `"labels":["A","B"]`) + assert.NotContains(t, tool.Description, `"x_column":"recorded_at"`) + + encoded := tool.RawInputSchema + require.NotEmpty(t, encoded) + for _, phrase := range []string{"workspace-relative CSV", "x-axis", "numeric column"} { + assert.Contains(t, string(encoded), phrase) + } + assert.Contains(t, string(encoded), `"examples"`) + assert.Contains(t, string(encoded), `"chart_type":"bar"`) + assert.Contains(t, string(encoded), `"chart_type":"line"`) + assert.Contains(t, string(encoded), `"labels":["A","B"]`) + assert.Contains(t, string(encoded), `"series":[{"label":"Count","values":[42,27]}]`) + require.NotNil(t, tool.Annotations.ReadOnlyHint) + assert.True(t, *tool.Annotations.ReadOnlyHint) + require.NotNil(t, tool.Annotations.DestructiveHint) + assert.False(t, *tool.Annotations.DestructiveHint) +} + +func TestRichOutputToolRejectsMismatchedChartSeries(t *testing.T) { + s := New(&testBackend{}, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + args := validRichOutputArgs() + chart := args["blocks"].([]interface{})[1].(map[string]interface{}) + series := chart["series"].([]interface{})[0].(map[string]interface{}) + series["values"] = []interface{}{18.2} + + result := callTool(t, s, richOutputToolName, args) + + require.True(t, result.IsError) + content, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Contains(t, content.Text, "chart series values must match labels") +} + +func TestRichOutputToolRejectsUnsafeFilePaths(t *testing.T) { + tests := []string{ + "/etc/passwd", + "../secrets.txt", + "reports/../../secrets.txt", + "https://example.com/report.json", + "data:text/plain,secret", + `C:\\Users\\person\\secret.txt`, + `\\\\server\\share\\secret.txt`, + } + + for _, filePath := range tests { + t.Run(filePath, func(t *testing.T) { + s := New(&testBackend{}, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + args := validRichOutputArgs() + file := args["blocks"].([]interface{})[2].(map[string]interface{}) + file["path"] = filePath + + result := callTool(t, s, richOutputToolName, args) + + require.True(t, result.IsError) + content, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Contains(t, content.Text, "file path must be workspace relative") + }) + } +} + +func TestRichOutputToolRejectsOversizedPayload(t *testing.T) { + labels := make([]interface{}, 100) + values := make([]interface{}, 100) + for i := range labels { + labels[i] = strings.Repeat("a", 120) + values[i] = 123456789012345.5 + } + blocks := make([]interface{}, 4) + for i := range blocks { + series := make([]interface{}, 4) + for j := range series { + series[j] = map[string]interface{}{ + "label": strings.Repeat("s", 120), + "values": values, + } + } + blocks[i] = map[string]interface{}{ + "type": "chart", + "chart_type": "bar", + "title": "Large chart", + "summary": "Bounded presentation", + "labels": labels, + "series": series, + } + } + args := map[string]interface{}{"version": 1, "title": "Too large", "blocks": blocks} + encoded, err := json.Marshal(args) + require.NoError(t, err) + require.Greater(t, len(encoded), 64*1024) + s := New(&testBackend{}, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + + result := callTool(t, s, richOutputToolName, args) + + require.True(t, result.IsError) + content, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Contains(t, content.Text, "rich output exceeds 64 KiB") +} + +func TestRichOutputToolRejectsSchemaViolations(t *testing.T) { + tests := []struct { + name string + mutate func(map[string]interface{}) + }{ + { + name: "unsupported version", + mutate: func(args map[string]interface{}) { + args["version"] = 2 + }, + }, + { + name: "unknown top-level field", + mutate: func(args map[string]interface{}) { + args["theme"] = "neon" + }, + }, + { + name: "no blocks", + mutate: func(args map[string]interface{}) { + args["blocks"] = []interface{}{} + }, + }, + { + name: "unknown block type", + mutate: func(args map[string]interface{}) { + args["blocks"] = []interface{}{map[string]interface{}{"type": "table"}} + }, + }, + { + name: "too many metrics", + mutate: func(args map[string]interface{}) { + items := make([]interface{}, 7) + for i := range items { + items[i] = map[string]interface{}{"label": "Metric", "value": "1"} + } + args["blocks"] = []interface{}{map[string]interface{}{"type": "metrics", "items": items}} + }, + }, + { + name: "too many chart labels", + mutate: func(args map[string]interface{}) { + labels := make([]interface{}, 101) + values := make([]interface{}, 101) + for i := range labels { + labels[i] = "run" + values[i] = i + } + args["blocks"] = []interface{}{map[string]interface{}{ + "type": "chart", + "chart_type": "line", + "title": "Runs", + "summary": "Run history", + "labels": labels, + "series": []interface{}{ + map[string]interface{}{"label": "Seconds", "values": values}, + }, + }} + }, + }, + { + name: "CSV chart with stray inline labels", + mutate: func(args map[string]interface{}) { + args["blocks"] = csvRichOutputArgs()["blocks"] + chart := args["blocks"].([]interface{})[0].(map[string]interface{}) + chart["labels"] = []interface{}{"run-1"} + }, + }, + { + name: "inline chart with stray CSV source", + mutate: func(args map[string]interface{}) { + chart := args["blocks"].([]interface{})[1].(map[string]interface{}) + chart["csv"] = csvChartSource(csvRichOutputArgs()) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + backend := &testBackend{} + s := New(backend, "session-1", "task-1", 10005, newTestLogger(t), "", false, ModeTask) + args := validRichOutputArgs() + tt.mutate(args) + + result := callTool(t, s, richOutputToolName, args) + + require.True(t, result.IsError) + assert.Empty(t, backend.lastAction) + }) + } +} + +func validRichOutputArgs() map[string]interface{} { + return map[string]interface{}{ + "version": 1, + "title": "Build health", + "description": "Latest local verification", + "blocks": []interface{}{ + map[string]interface{}{ + "type": "metrics", + "items": []interface{}{ + map[string]interface{}{"label": "Passed", "value": "38"}, + }, + }, + map[string]interface{}{ + "type": "chart", + "chart_type": "line", + "title": "Runtime by run", + "summary": "Runtime fell across the last three runs.", + "labels": []interface{}{"1", "2", "3"}, + "series": []interface{}{ + map[string]interface{}{"label": "Seconds", "values": []interface{}{18.2, 16.4, nil}}, + }, + }, + map[string]interface{}{ + "type": "file", + "path": "reports/build.json", + "repo": "kandev", + "title": "Raw report", + "caption": "Generated by verification", + "mime_type": "application/json", + }, + }, + } +} + +func csvRichOutputArgs() map[string]interface{} { + return map[string]interface{}{ + "version": 1, + "title": "API latency", + "blocks": []interface{}{ + map[string]interface{}{ + "type": "chart", + "chart_type": "line", + "title": "Latency over time", + "summary": "p50 and p95 latency from the workspace report.", + "csv": map[string]interface{}{ + "path": "reports/latency.csv", + "repo": "api", + "x_column": "recorded_at", + "series": []interface{}{ + map[string]interface{}{"column": "p50_ms", "label": "p50"}, + map[string]interface{}{"column": "p95_ms"}, + }, + }, + }, + }, + } +} + +func csvChartSource(args map[string]interface{}) map[string]interface{} { + chart := args["blocks"].([]interface{})[0].(map[string]interface{}) + return chart["csv"].(map[string]interface{}) +} + +func assertJSONEquivalent(t *testing.T, want, got interface{}) { + t.Helper() + normalize := func(value interface{}) interface{} { + encoded, err := json.Marshal(value) + require.NoError(t, err) + var normalized interface{} + require.NoError(t, json.Unmarshal(encoded, &normalized)) + return normalized + } + assert.Equal(t, normalize(want), normalize(got)) +} diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index a52f98bb59..476cfac9fb 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -909,6 +909,7 @@ func (s *Server) profileToolGroups() []profileToolGroup { {name: "user-question", enabled: capabilityEnabled(mcpprofile.CapabilityUserQuestion), register: func(s *Server) { s.registerInteractionTools() }}, {name: "parent-question", enabled: andProfilePredicates(kanban, capabilityEnabled(mcpprofile.CapabilityParentQuestion)), register: func(s *Server) { s.registerParentQuestionTool() }}, {name: "plan", enabled: func(ctx mcpprofile.Context) bool { return kanban(ctx) || office(ctx) }, register: func(s *Server) { s.registerPlanTools() }}, + {name: "rich-output", enabled: func(ctx mcpprofile.Context) bool { return kanban(ctx) || office(ctx) }, register: func(s *Server) { s.registerRichOutputTool() }}, {name: "walkthrough", enabled: kanban, register: func(s *Server) { s.registerWalkthroughTools() }}, {name: "review", enabled: kanban, register: func(s *Server) { s.registerReviewTools() }}, {name: "related-tasks", enabled: func(ctx mcpprofile.Context) bool { return kanban(ctx) || office(ctx) }, register: func(s *Server) { s.registerRelatedTasksTool() }}, diff --git a/apps/backend/internal/mcp/server/server_test.go b/apps/backend/internal/mcp/server/server_test.go index 54edaab392..246d95a0fc 100644 --- a/apps/backend/internal/mcp/server/server_test.go +++ b/apps/backend/internal/mcp/server/server_test.go @@ -709,7 +709,7 @@ drained: // as in TestServerModeTask_ToolCount and // TestRegisterTools_LoggedCountMatchesRegisteredTools (list_task_sessions_test.go), // which pin the per-mode registration rather than this SetProviders rebuild. - require.Len(t, tools, 35, "final registry should contain the complete GitLab-only task tool set") + require.Len(t, tools, 36, "final registry should contain the complete GitLab-only task tool set") assert.Contains(t, tools, "get_task_mr_automation_kandev") assert.NotContains(t, tools, "get_task_pr_automation_kandev") } @@ -878,7 +878,7 @@ func TestServerModeTask_ToolCount(t *testing.T) { // 1 add_workspace_sources + 1 update_repository_base_branch + // 1 step_complete (ADR 0015) + 1 interaction + 4 plan + 3 walkthrough + // 1 publish_review_findings + 1 related-tasks + 1 diagnostic bundle - // + 2 task-dependency (add/remove) = 37. + // + 2 task-dependency (add/remove) + 1 rich-output = 38. // Task-document tools (list/get/write) are office-only. assert.Contains(t, tools, "step_complete_kandev", "ADR 0015 explicit-completion signal must be registered in task mode") assert.Contains(t, tools, "show_walkthrough_kandev", "walkthrough tool must be registered in task mode") @@ -888,7 +888,8 @@ func TestServerModeTask_ToolCount(t *testing.T) { assert.Contains(t, tools, "list_task_sessions_kandev", "session discovery must be registered in task mode") assert.Contains(t, tools, "add_task_dependency_kandev", "dependency edges must be manageable in task mode") assert.Contains(t, tools, "remove_task_dependency_kandev") - assert.Equal(t, 37, len(tools)) + assert.Contains(t, tools, "show_rich_output_kandev", "native rich output must be registered in task mode") + assert.Equal(t, 38, len(tools)) } func TestServerStepCompleteTool_TaskOnlyAndDiscoverable(t *testing.T) { @@ -958,6 +959,7 @@ func TestServerModeOffice_RegistersCorrectTools(t *testing.T) { // Office mode should have interaction tools assert.Contains(t, tools, "ask_user_question_kandev") + assert.Contains(t, tools, "show_rich_output_kandev") // delegate_task_kandev was removed from ModeOffice when the // agentctl CLI started covering task creation/delegation via @@ -994,10 +996,10 @@ func TestServerModeOffice_ToolCount(t *testing.T) { s := New(backend, "test-session", "test-task", 10005, log, "", false, ModeOffice) tools := getRegisteredToolNames(s) - // 4 plan + 1 interaction + 1 related-tasks + 3 task-documents = 9 + // 4 plan + 1 interaction + 1 related-tasks + 3 task-documents + 1 rich-output = 10 // (delegate_task_kandev retired in favour of `agentctl kandev task create …`). assert.NotContains(t, tools, "step_complete_kandev", "step_complete_kandev is kanban-task-only; office mode advances tasks via its own approval surface") - assert.Equal(t, 9, len(tools)) + assert.Equal(t, 10, len(tools)) } func TestServerModeOffice_DisableAskQuestion(t *testing.T) { @@ -1014,8 +1016,8 @@ func TestServerModeOffice_DisableAskQuestion(t *testing.T) { // delegate_task_kandev was retired from ModeOffice (now lives in // the agentctl CLI as `agentctl kandev task create --parent …`). assert.NotContains(t, tools, "delegate_task_kandev") - // 4 plan + 1 related-tasks + 3 task-documents = 8 (no ask_user_question, no delegate) - assert.Equal(t, 8, len(tools)) + // 4 plan + 1 related-tasks + 3 task-documents + 1 rich-output = 9 (no ask_user_question, no delegate) + assert.Equal(t, 9, len(tools)) } func TestServerModeConstants(t *testing.T) { diff --git a/apps/backend/internal/mcp/server/sysprompt_sync_test.go b/apps/backend/internal/mcp/server/sysprompt_sync_test.go index eeaf7243f1..c0ba81861a 100644 --- a/apps/backend/internal/mcp/server/sysprompt_sync_test.go +++ b/apps/backend/internal/mcp/server/sysprompt_sync_test.go @@ -7,6 +7,7 @@ import ( "sort" "strings" "testing" + "unicode/utf8" promptcfg "github.com/kandev/kandev/config/prompts" "github.com/kandev/kandev/internal/sysprompt" @@ -172,6 +173,47 @@ func TestTaskControlDocs_OmittedFromRestrictedModes(t *testing.T) { } } +func TestRichOutputDocs_MakeExplicitAndCSVUseDiscoverable(t *testing.T) { + inlineRecipe := `{"type":"chart","chart_type":"bar","title":"T","summary":"S","labels":["A","B"],"series":[{"label":"Count","values":[42,27]}]}` + csvRecipe := `{"type":"chart","chart_type":"line","title":"T","summary":"S","csv":{"path":"reports/latency.csv","x_column":"recorded_at","series":[{"column":"p95_ms","label":"p95 (ms)"}]}}` + metricsRecipe := `{"type":"metrics","items":[{"label":"Passed","value":"38"}]}` + + for name, prompt := range map[string]string{ + "task": sysprompt.KandevContext(), + "office": sysprompt.OfficeContext(), + } { + t.Run(name, func(t *testing.T) { + var richOutputLine string + for _, line := range strings.Split(prompt, "\n") { + if strings.HasPrefix(line, "- show_rich_output_kandev:") { + richOutputLine = line + break + } + } + require.NotEmpty(t, richOutputLine) + assert.LessOrEqual(t, utf8.RuneCountInString(richOutputLine), 750) + for _, phrase := range []string{ + "When user asks for chart/graph/plot/file preview/KPI/metrics with data: call now", + "Do not implement the display as ASCII/SVG/HTML or with another app", + "version=1,title,blocks (1-4)", + "x_column", + "workspace-relative", + "Label series with units", + "small text table: Markdown", + "Inline:", + "CSV line:", + "Metrics:", + "Kandev owns axes/legends/tooltips/layout", + } { + assert.Contains(t, richOutputLine, phrase) + } + assert.Equal(t, 1, strings.Count(richOutputLine, inlineRecipe)) + assert.Equal(t, 1, strings.Count(richOutputLine, csvRecipe)) + assert.Equal(t, 1, strings.Count(richOutputLine, metricsRecipe)) + }) + } +} + // TestSyspromptToolNames_MatchMCPConfigMode verifies that every `_kandev` // tool referenced in ConfigContext is registered by an MCP server in ModeConfig. func TestSyspromptToolNames_MatchMCPConfigMode(t *testing.T) { diff --git a/apps/backend/internal/mcp/server/tool_metadata_test.go b/apps/backend/internal/mcp/server/tool_metadata_test.go index 9e6fc9d0ac..2e84a514c2 100644 --- a/apps/backend/internal/mcp/server/tool_metadata_test.go +++ b/apps/backend/internal/mcp/server/tool_metadata_test.go @@ -21,6 +21,7 @@ func TestCoreToolDescriptionsStayFocused(t *testing.T) { "add_branch_to_task_kandev": 650, "step_complete_kandev": 650, "ask_user_question_kandev": 600, + "show_rich_output_kandev": 650, "show_walkthrough_kandev": 650, "publish_review_findings_kandev": 650, "update_repository_base_branch_kandev": 500, diff --git a/apps/web/app/office/tasks/[id]/advanced-panels/chat-panel.tsx b/apps/web/app/office/tasks/[id]/advanced-panels/chat-panel.tsx index f656a02a57..2468329177 100644 --- a/apps/web/app/office/tasks/[id]/advanced-panels/chat-panel.tsx +++ b/apps/web/app/office/tasks/[id]/advanced-panels/chat-panel.tsx @@ -74,7 +74,7 @@ function MessageList({ taskId: string; sessionId: string | null; worktreePath?: string; - onOpenFile?: (path: string) => void; + onOpenFile?: (path: string, repo?: string) => void; activeTurnId: string | null; scrollRef: React.RefObject; }) { diff --git a/apps/web/components/settings/appearance-account-sections.tsx b/apps/web/components/settings/appearance-account-sections.tsx new file mode 100644 index 0000000000..66492d4bf5 --- /dev/null +++ b/apps/web/components/settings/appearance-account-sections.tsx @@ -0,0 +1,118 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { IconActivity, IconGitBranch } from "@tabler/icons-react"; +import { CardContent, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Label } from "@kandev/ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; +import { Separator } from "@kandev/ui/separator"; +import { GENERAL_SETTINGS_TARGETS } from "@/lib/settings-discovery/catalog/preferences"; +import type { AppearanceState } from "./appearance-settings-state"; +import { AppStatusBarSettingsCard } from "./app-status-bar-settings-card"; +import { SettingsCard } from "./settings-card"; +import { SettingsSection } from "./settings-section"; +import { SystemMetricsSettingsCard } from "./system-metrics-settings-card"; + +function ChangesPanelLayoutCard({ + value, + isDirty, + onChange, +}: { + value: "flat" | "tree"; + isDirty: boolean; + onChange: (value: "flat" | "tree") => void; +}) { + const { t } = useTranslation(); + return ( + + + {t("settings:changesPanelLayout")} + + +
+ + +

+ {t("settings:displayChangedFilesAsAFlat")} +

+
+ + + ); +} + +export function AppearanceAccountSections({ + draft, + saved, + updateDraft, +}: { + draft: AppearanceState; + saved: AppearanceState; + updateDraft: (patch: Partial) => void; +}) { + const { t } = useTranslation(); + return ( + <> + + } + title={t("settings:statusBar")} + description={t("settings:configureStatusBarVisibility")} + > + updateDraft({ appStatusBarEnabled })} + /> + + + + + } + title={t("settings:changesPanel")} + description={t("settings:customizeHowChangedFilesAreDisplayed")} + > + updateDraft({ changesPanelLayout })} + /> + + + + + } + title={t("settings:resourceMetrics")} + description={t("settings:configureBackendAndExecutionResourceSampling")} + > + updateDraft({ showMetrics })} + simplified={draft.simplifiedMetrics} + isSimplifiedDirty={draft.simplifiedMetrics !== saved.simplifiedMetrics} + onSimplifiedChange={(simplifiedMetrics) => updateDraft({ simplifiedMetrics })} + /> + + + ); +} diff --git a/apps/web/components/settings/appearance-settings-state.ts b/apps/web/components/settings/appearance-settings-state.ts index 3581e4861f..b7213bcf95 100644 --- a/apps/web/components/settings/appearance-settings-state.ts +++ b/apps/web/components/settings/appearance-settings-state.ts @@ -6,6 +6,7 @@ import type { UserSettingsUpdatePayload } from "@/lib/types/http"; export type AppearanceState = { theme: Theme; settingsMenuMode: SettingsMenuMode; + richOutputAnimationsEnabled: boolean; startupPage: UserSettingsState["startupPage"]; changesPanelLayout: UserSettingsState["changesPanelLayout"]; appStatusBarEnabled: boolean; @@ -16,6 +17,7 @@ export type AppearanceState = { export function createAppearanceSavedState( theme: Theme, settingsMenuMode: SettingsMenuMode, + richOutputAnimationsEnabled: boolean, userSettings: Pick< UserSettingsState, "appStatusBarEnabled" | "changesPanelLayout" | "startupPage" | "systemMetricsDisplay" @@ -26,6 +28,7 @@ export function createAppearanceSavedState( appStatusBarEnabled: userSettings.appStatusBarEnabled, // Per-device, but drafted and saved with account settings under one control. settingsMenuMode, + richOutputAnimationsEnabled, changesPanelLayout: userSettings.changesPanelLayout, startupPage: userSettings.startupPage, showMetrics: userSettings.systemMetricsDisplay.showInTopbar, @@ -73,6 +76,7 @@ export function rebaseAppearanceDraft( return { theme: rebase("theme"), settingsMenuMode: rebase("settingsMenuMode"), + richOutputAnimationsEnabled: rebase("richOutputAnimationsEnabled"), startupPage: rebase("startupPage"), changesPanelLayout: rebase("changesPanelLayout"), appStatusBarEnabled: rebase("appStatusBarEnabled"), @@ -85,6 +89,7 @@ export function appearanceRevision(state: AppearanceState): string { return JSON.stringify([ state.theme, state.settingsMenuMode, + state.richOutputAnimationsEnabled, state.startupPage, state.changesPanelLayout, state.appStatusBarEnabled, diff --git a/apps/web/components/settings/general-settings.test.tsx b/apps/web/components/settings/general-settings.test.tsx index 601a308548..6a192baf7f 100644 --- a/apps/web/components/settings/general-settings.test.tsx +++ b/apps/web/components/settings/general-settings.test.tsx @@ -6,9 +6,11 @@ import { AppearanceSettings } from "./general-settings"; const apiMocks = vi.hoisted(() => ({ updateUserSettings: vi.fn() })); const SHOW_STATUS_BAR_LABEL = "Show status bar"; +const ANIMATE_RICH_OUTPUT_CHARTS_LABEL = "Animate rich-output charts"; const SAVE_CHANGES_LABEL = "Save changes"; const CHECKED_STATE = "checked"; const DATA_STATE_ATTRIBUTE = "data-state"; +const DATA_SETTINGS_DIRTY_ATTRIBUTE = "data-settings-dirty"; const INITIAL_REVISION = 1; const OLDER_LIVE_REVISION = 2; const SAVED_REVISION = 3; @@ -23,6 +25,9 @@ const storeMocks = vi.hoisted(() => ({ previewSettingsMenuMode: vi.fn(), commitSettingsMenuMode: vi.fn(), restoreSettingsMenuMode: vi.fn(), + previewRichOutputAnimations: vi.fn(), + commitRichOutputAnimations: vi.fn(), + restoreRichOutputAnimations: vi.fn(), })); vi.mock("@/lib/api", () => ({ @@ -183,7 +188,7 @@ async function verifyEditDuringSaveSurvivesItsLiveUpdate() { resolveSave(appearanceSettingsResponse({ app_status_bar_enabled: false })); await waitFor(() => expect(storeMocks.setUserSettings).toHaveBeenCalledOnce()); expect(toggle.getAttribute(DATA_STATE_ATTRIBUTE)).toBe(CHECKED_STATE); - expect(toggle.getAttribute("data-settings-dirty")).toBe("true"); + expect(toggle.getAttribute(DATA_SETTINGS_DIRTY_ATTRIBUTE)).toBe("true"); } async function verifyNewerSaveResponseWinsOverOlderLiveUpdate() { @@ -241,21 +246,58 @@ beforeEach(() => { storeMocks.previewSettingsMenuMode.mockReset(); storeMocks.commitSettingsMenuMode.mockReset(); storeMocks.restoreSettingsMenuMode.mockReset(); + storeMocks.previewRichOutputAnimations.mockReset(); + storeMocks.commitRichOutputAnimations.mockReset(); + storeMocks.restoreRichOutputAnimations.mockReset(); storeMocks.state = { userSettings: { ...defaultSettingsState.userSettings, appStatusBarEnabled: true, }, settingsMenu: { savedMode: "flat" }, + richOutputMotion: { enabled: true, savedEnabled: true }, setUserSettings: storeMocks.setUserSettings, previewSettingsMenuMode: storeMocks.previewSettingsMenuMode, commitSettingsMenuMode: storeMocks.commitSettingsMenuMode, restoreSettingsMenuMode: storeMocks.restoreSettingsMenuMode, + previewRichOutputAnimations: storeMocks.previewRichOutputAnimations, + commitRichOutputAnimations: storeMocks.commitRichOutputAnimations, + restoreRichOutputAnimations: storeMocks.restoreRichOutputAnimations, }; }); afterEach(cleanup); +describe("AppearanceSettings rich-output motion preference", () => { + it("previews and saves chart motion locally without a user-settings request", async () => { + renderAppearance(); + + const toggle = screen.getByRole("switch", { name: ANIMATE_RICH_OUTPUT_CHARTS_LABEL }); + expect(toggle.getAttribute(DATA_STATE_ATTRIBUTE)).toBe(CHECKED_STATE); + + fireEvent.click(toggle); + expect(storeMocks.previewRichOutputAnimations).toHaveBeenCalledWith(false); + expect(toggle.getAttribute(DATA_SETTINGS_DIRTY_ATTRIBUTE)).toBe("true"); + + fireEvent.click(await screen.findByRole("button", { name: SAVE_CHANGES_LABEL })); + + await waitFor(() => expect(storeMocks.commitRichOutputAnimations).toHaveBeenCalledWith(false)); + expect(apiMocks.updateUserSettings).not.toHaveBeenCalled(); + expect(storeMocks.setUserSettings).not.toHaveBeenCalled(); + }); + + it("restores the saved chart motion preference through Reset", async () => { + renderAppearance(); + + const toggle = screen.getByRole("switch", { name: ANIMATE_RICH_OUTPUT_CHARTS_LABEL }); + fireEvent.click(toggle); + fireEvent.click(await screen.findByRole("button", { name: "Reset" })); + + await waitFor(() => expect(toggle.getAttribute(DATA_STATE_ATTRIBUTE)).toBe(CHECKED_STATE)); + expect(storeMocks.restoreRichOutputAnimations).toHaveBeenCalledOnce(); + }); +}); + describe("AppearanceSettings status bar preference", () => { it("saves local-only Appearance changes without a user-settings request", async () => { renderAppearance(); @@ -288,7 +330,7 @@ describe("AppearanceSettings status bar preference", () => { expect(apiMocks.updateUserSettings).not.toHaveBeenCalled(); expect(storeMocks.setUserSettings).not.toHaveBeenCalled(); - expect(toggle.getAttribute("data-settings-dirty")).toBe("true"); + expect(toggle.getAttribute(DATA_SETTINGS_DIRTY_ATTRIBUTE)).toBe("true"); fireEvent.click(await screen.findByRole("button", { name: SAVE_CHANGES_LABEL })); @@ -328,7 +370,7 @@ describe("AppearanceSettings status bar preference", () => { expect(await screen.findByText("Couldn't save")).toBeTruthy(); expect(toggle.getAttribute(DATA_STATE_ATTRIBUTE)).toBe("unchecked"); - expect(toggle.getAttribute("data-settings-dirty")).toBe("true"); + expect(toggle.getAttribute(DATA_SETTINGS_DIRTY_ATTRIBUTE)).toBe("true"); expect(storeMocks.setUserSettings).not.toHaveBeenCalled(); expect( (storeMocks.state.userSettings as { appStatusBarEnabled: boolean }).appStatusBarEnabled, diff --git a/apps/web/components/settings/general-settings.tsx b/apps/web/components/settings/general-settings.tsx index dfbfb9b31d..7d852cabd0 100644 --- a/apps/web/components/settings/general-settings.tsx +++ b/apps/web/components/settings/general-settings.tsx @@ -3,12 +3,10 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { useTheme } from "@/components/theme/app-theme"; import { - IconActivity, IconBinaryTree, IconCommand, IconPalette, IconKeyboard, - IconGitBranch, IconArchive, IconArrowBackUp, IconListCheck, @@ -24,7 +22,6 @@ import { SettingsCardHeader } from "@/components/settings/settings-card-header"; import { settingsControlClassName } from "@/components/settings/settings-control"; import { SettingsPageHeader } from "@/components/settings/settings-typography"; import { KeyboardShortcutsCard } from "@/components/settings/keyboard-shortcuts-card"; -import { SystemMetricsSettingsCard } from "@/components/settings/system-metrics-settings-card"; import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { updateUserSettings } from "@/lib/api"; import type { Theme } from "@/lib/settings/types"; @@ -44,8 +41,9 @@ import { usePlugins } from "@/hooks/domains/plugins/use-plugins"; import { StartupPageSettingsCard } from "@/components/settings/startup-page-settings-card"; import { GENERAL_SETTINGS_TARGETS } from "@/lib/settings-discovery/catalog/preferences"; import { SleepInhibitionSettings } from "@/components/settings/sleep-inhibition-settings"; -import { AppStatusBarSettingsCard } from "@/components/settings/app-status-bar-settings-card"; import { SettingsMenuModeCard } from "@/components/settings/settings-menu-mode-card"; +import { RichOutputMotionSettingsCard } from "@/components/settings/rich-output-motion-settings-card"; +import { AppearanceAccountSections } from "@/components/settings/appearance-account-sections"; import type { SettingsMenuMode } from "@/lib/settings/settings-menu-mode"; import { mapUserSettingsResponse } from "@/lib/ssr/user-settings"; import { compareUserSettingsRevisions } from "@/lib/settings/user-settings-revision"; @@ -138,51 +136,6 @@ function ChatSubmitKeyCard({ ); } -function ChangesPanelLayoutCard({ - value, - isDirty, - onChange, -}: { - value: "flat" | "tree"; - isDirty: boolean; - onChange: (value: "flat" | "tree") => void; -}) { - const { t } = useTranslation(); - return ( - - - {t("settings:changesPanelLayout")} - - -
- - -

- {t("settings:displayChangedFilesAsAFlat")} -

-
- - - ); -} - function StartupPageSettingsSection({ value, isDirty, @@ -237,12 +190,18 @@ function SettingsMenuModeSection({ function AppearanceThemeSection({ theme, - isDirty, - onChange, + isThemeDirty, + onThemeChange, + richOutputAnimationsEnabled, + isRichOutputMotionDirty, + onRichOutputMotionChange, }: { theme: Theme; - isDirty: boolean; - onChange: (theme: Theme) => void; + isThemeDirty: boolean; + onThemeChange: (theme: Theme) => void; + richOutputAnimationsEnabled: boolean; + isRichOutputMotionDirty: boolean; + onRichOutputMotionChange: (enabled: boolean) => void; }) { const { t } = useTranslation(); return ( @@ -251,35 +210,18 @@ function AppearanceThemeSection({ title={t("settings:appearance")} description={t("settings:customizeHowTheApplicationLooks")} > - +
+ + +
); } -function AppStatusBarSettingsSection({ - enabled, - isDirty, - onChange, -}: { - enabled: boolean; - isDirty: boolean; - onChange: (enabled: boolean) => void; -}) { - const { t } = useTranslation(); - return ( - <> - - } - title={t("settings:statusBar")} - description={t("settings:configureStatusBarVisibility")} - > - - - - ); -} - export function TaskActionsSettings() { const { t } = useTranslation(); return ( @@ -352,6 +294,9 @@ function useAppearanceSaveContributor({ const previewMenuMode = useAppStore((state) => state.previewSettingsMenuMode); const commitMenuMode = useAppStore((state) => state.commitSettingsMenuMode); const restoreMenuMode = useAppStore((state) => state.restoreSettingsMenuMode); + const previewRichOutputAnimations = useAppStore((state) => state.previewRichOutputAnimations); + const commitRichOutputAnimations = useAppStore((state) => state.commitRichOutputAnimations); + const restoreRichOutputAnimations = useAppStore((state) => state.restoreRichOutputAnimations); const revision = appearanceRevision(draft); useSettingsSaveContributor({ @@ -369,6 +314,7 @@ function useAppearanceSaveContributor({ const response = Object.keys(patch).length > 0 ? await updateUserSettings(patch) : null; commitTheme(submitted.theme); commitMenuMode(submitted.settingsMenuMode); + commitRichOutputAnimations(submitted.richOutputAnimationsEnabled); // A draft edited while the save was in flight keeps its preview: what was // submitted is now persisted, but the screen should still show what the // user is currently looking at. @@ -378,6 +324,11 @@ function useAppearanceSaveContributor({ if (draftRef.current.settingsMenuMode !== submitted.settingsMenuMode) { previewMenuMode(draftRef.current.settingsMenuMode); } + if ( + draftRef.current.richOutputAnimationsEnabled !== submitted.richOutputAnimationsEnabled + ) { + previewRichOutputAnimations(draftRef.current.richOutputAnimationsEnabled); + } const latestUserSettings = storeApi.getState().userSettings; let responseIsCurrent = false; if (response) { @@ -395,6 +346,7 @@ function useAppearanceSaveContributor({ const confirmed = createAppearanceSavedState( submitted.theme, submitted.settingsMenuMode, + submitted.richOutputAnimationsEnabled, nextUserSettings, ); setSaved(confirmed); @@ -410,6 +362,7 @@ function useAppearanceSaveContributor({ setDraft(saved); restoreTheme(); restoreMenuMode(); + restoreRichOutputAnimations(); }, }); } @@ -439,23 +392,32 @@ function AppearanceSettingsSections({ updateDraft, previewTheme, previewMenuMode, + previewRichOutputAnimations, }: { draft: AppearanceState; saved: AppearanceState; updateDraft: (patch: Partial) => void; previewTheme: (theme: Theme) => void; previewMenuMode: (mode: SettingsMenuMode) => void; + previewRichOutputAnimations: (enabled: boolean) => void; }) { - const { t } = useTranslation(); return ( <> { + isThemeDirty={draft.theme !== saved.theme} + onThemeChange={(theme) => { updateDraft({ theme }); previewTheme(theme); }} + richOutputAnimationsEnabled={draft.richOutputAnimationsEnabled} + isRichOutputMotionDirty={ + draft.richOutputAnimationsEnabled !== saved.richOutputAnimationsEnabled + } + onRichOutputMotionChange={(richOutputAnimationsEnabled) => { + updateDraft({ richOutputAnimationsEnabled }); + previewRichOutputAnimations(richOutputAnimationsEnabled); + }} /> - - updateDraft({ appStatusBarEnabled })} - /> - - - - } - title={t("settings:changesPanel")} - description={t("settings:customizeHowChangedFilesAreDisplayed")} - > - updateDraft({ changesPanelLayout })} - /> - - - - - } - title={t("settings:resourceMetrics")} - description={t("settings:configureBackendAndExecutionResourceSampling")} - > - updateDraft({ showMetrics })} - simplified={draft.simplifiedMetrics} - isSimplifiedDirty={draft.simplifiedMetrics !== saved.simplifiedMetrics} - onSimplifiedChange={(simplifiedMetrics) => updateDraft({ simplifiedMetrics })} - /> - + ); } @@ -522,9 +448,11 @@ export function AppearanceSettings() { const userSettings = useAppStore((state) => state.userSettings); const { savedTheme, previewTheme } = useTheme(); const savedMenuMode = useAppStore((state) => state.settingsMenu.savedMode); + const savedRichOutputAnimations = useAppStore((state) => state.richOutputMotion.savedEnabled); const previewMenuMode = useAppStore((state) => state.previewSettingsMenuMode); + const previewRichOutputAnimations = useAppStore((state) => state.previewRichOutputAnimations); const [saved, setSaved] = useState(() => - createAppearanceSavedState(savedTheme, savedMenuMode, userSettings), + createAppearanceSavedState(savedTheme, savedMenuMode, savedRichOutputAnimations, userSettings), ); const [draft, setDraft] = useState(saved); const draftRef = useRef(draft); @@ -538,6 +466,7 @@ export function AppearanceSettings() { const nextSaved = createAppearanceSavedState( previousSaved.theme, previousSaved.settingsMenuMode, + previousSaved.richOutputAnimationsEnabled, userSettings, ); setDraft( @@ -575,6 +504,7 @@ export function AppearanceSettings() { updateDraft={updateDraft} previewTheme={previewTheme} previewMenuMode={previewMenuMode} + previewRichOutputAnimations={previewRichOutputAnimations} /> ); diff --git a/apps/web/components/settings/rich-output-motion-settings-card.test.tsx b/apps/web/components/settings/rich-output-motion-settings-card.test.tsx new file mode 100644 index 0000000000..aac3f25ef4 --- /dev/null +++ b/apps/web/components/settings/rich-output-motion-settings-card.test.tsx @@ -0,0 +1,37 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { i18n } from "@/lib/i18n"; +import { RichOutputMotionSettingsCard } from "./rich-output-motion-settings-card"; + +afterEach(async () => { + cleanup(); + await i18n.changeLanguage("en"); +}); + +describe("RichOutputMotionSettingsCard", () => { + it("renders a translated, controlled, touch-sized device preference", async () => { + await i18n.changeLanguage("pseudo"); + const onChange = vi.fn(); + render(); + + const toggle = screen.getByRole("switch", { name: "Àńĩḿàţē ŕĩćĥ-ōũţƥũţ ćĥàŕţś" }); + expect(toggle.getAttribute("data-state")).toBe("checked"); + expect(toggle.getAttribute("data-settings-dirty")).toBe("true"); + expect(screen.getByTestId("rich-output-motion-settings-card").dataset.settingsDirty).toBe( + "true", + ); + expect(screen.getByTestId("rich-output-motion-toggle-row").className).toContain("min-h-11"); + expect(toggle.className).toContain("data-[size=default]:h-11"); + expect(toggle.className).toContain("data-[size=default]:w-11"); + expect(toggle.className).toContain("data-[state=checked]:before:bg-primary"); + expect( + screen.getByText( + "Àńĩḿàţē ĺĩńē àńď ƀàŕ ćĥàŕţś ŵĥēń àĝēńţś ƥŕēśēńţ ţĥēḿ. Ţũŕń ţĥĩś ōƒƒ ţō ŕēďũćē ƀŕōŵśēŕ ŵōŕķ; ćĥàŕţ ďàţà àńď ćōńţŕōĺś śţàŷ àvàĩĺàƀĺē. Ŷōũŕ ōƥēŕàţĩńĝ śŷśţēḿ'ś ŕēďũćēď-ḿōţĩōń śēţţĩńĝ àĺŵàŷś ţàķēś ƥŕĩōŕĩţŷ. Śàvēď ōń ţĥĩś ďēvĩćē.", + ), + ).toBeTruthy(); + + fireEvent.click(toggle); + expect(onChange).toHaveBeenCalledWith(false); + expect(onChange).toHaveBeenCalledOnce(); + }); +}); diff --git a/apps/web/components/settings/rich-output-motion-settings-card.tsx b/apps/web/components/settings/rich-output-motion-settings-card.tsx new file mode 100644 index 0000000000..c3b1ac594e --- /dev/null +++ b/apps/web/components/settings/rich-output-motion-settings-card.tsx @@ -0,0 +1,53 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { CardContent, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Label } from "@kandev/ui/label"; +import { Switch } from "@kandev/ui/switch"; +import { GENERAL_SETTINGS_TARGETS } from "@/lib/settings-discovery/catalog/preferences"; +import { SettingsCard } from "./settings-card"; + +export function RichOutputMotionSettingsCard({ + enabled, + isDirty, + onChange, +}: { + enabled: boolean; + isDirty: boolean; + onChange: (enabled: boolean) => void; +}) { + const { t } = useTranslation(); + return ( + + + {t("settings:richOutputChartMotion")} + + +
+
+ +

+ {t("settings:animateRichOutputChartsDescription")} +

+
+ +
+ + + ); +} diff --git a/apps/web/components/task/chat/message-list-native.tsx b/apps/web/components/task/chat/message-list-native.tsx index d0b72ce2d3..6505f83b86 100644 --- a/apps/web/components/task/chat/message-list-native.tsx +++ b/apps/web/components/task/chat/message-list-native.tsx @@ -162,7 +162,7 @@ type MessageRowProps = { childrenByParentToolCallId: Map; taskId?: string; worktreePath?: string; - onOpenFile?: (path: string) => void; + onOpenFile?: (path: string, repo?: string) => void; isLastGroup: boolean; activeTurnId: string | null; streamingMessageId: string | null; @@ -223,7 +223,7 @@ type NativeMessageListBodyProps = { messagesLoading: boolean; sessionState?: TaskSessionState; worktreePath?: string; - onOpenFile?: (path: string) => void; + onOpenFile?: (path: string, repo?: string) => void; hasMore: boolean; isLoadingMore: boolean; isInitialLoading: boolean; diff --git a/apps/web/components/task/chat/message-list-shared.tsx b/apps/web/components/task/chat/message-list-shared.tsx index 3b632682be..1d86907c01 100644 --- a/apps/web/components/task/chat/message-list-shared.tsx +++ b/apps/web/components/task/chat/message-list-shared.tsx @@ -32,7 +32,7 @@ export type MessageListProps = { isWorking: boolean; sessionState?: TaskSessionState; worktreePath?: string; - onOpenFile?: (path: string) => void; + onOpenFile?: (path: string, repo?: string) => void; /** Render item key (see getItemKey) the unread "New" divider should * appear immediately before; null/undefined renders no divider. */ dividerBeforeItemKey?: string | null; @@ -472,7 +472,7 @@ export const MessageItem = memo(function MessageItem({ childrenByParentToolCallId: Map; taskId?: string; worktreePath?: string; - onOpenFile?: (path: string) => void; + onOpenFile?: (path: string, repo?: string) => void; isLastGroup: boolean; activeTurnId?: string | null; streamingMessageId?: string | null; diff --git a/apps/web/components/task/chat/message-renderer.tsx b/apps/web/components/task/chat/message-renderer.tsx index b048b169f8..9ba82de42a 100644 --- a/apps/web/components/task/chat/message-renderer.tsx +++ b/apps/web/components/task/chat/message-renderer.tsx @@ -41,7 +41,7 @@ type AdapterContext = { childrenByParentToolCallId?: Map; worktreePath?: string; sessionId?: string; - onOpenFile?: (path: string) => void; + onOpenFile?: (path: string, repo?: string) => void; onScrollToMessage?: (messageId: string) => void; isTurnActive?: boolean; isContainingTurnActive?: boolean; @@ -331,7 +331,14 @@ const adapters: MessageAdapter[] = [ const permissionMessage = toolCallId ? ctx.permissionsByToolCallId?.get(toolCallId) : undefined; - return ; + return ( + + ); }, }, { @@ -440,7 +447,7 @@ type MessageRendererProps = { childrenByParentToolCallId?: Map; worktreePath?: string; sessionId?: string; - onOpenFile?: (path: string) => void; + onOpenFile?: (path: string, repo?: string) => void; onScrollToMessage?: (messageId: string) => void; isTurnActive?: boolean; isContainingTurnActive?: boolean; diff --git a/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx b/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx index e2e5be7d79..17d17e8a58 100644 --- a/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx +++ b/apps/web/components/task/chat/messages/kandev-tool-message.test.tsx @@ -1,7 +1,8 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; import { KandevToolMessage, hasKandevRenderer } from "./kandev-tool-message"; import { sessionId as toSessionId, taskId as toTaskId, type Message } from "@/lib/types/http"; +import { KANDEV_RENDERERS } from "./kandev/registry"; // kandevToolCall constructs the kandev Message shape produced by the // orchestrator for an MCP tool_call. The shape mirrors live production data @@ -54,6 +55,9 @@ describe("hasKandevRenderer", () => { expect( hasKandevRenderer(kandevToolCall({ toolName: "mcp__kandev__show_walkthrough_kandev" })), ).toBe(true); + expect( + hasKandevRenderer(kandevToolCall({ toolName: "mcp__kandev__show_rich_output_kandev" })), + ).toBe(true); }); it("does not match unrelated tools", () => { @@ -68,6 +72,95 @@ describe("hasKandevRenderer", () => { }); }); +describe("KandevToolMessage host context", () => { + it("passes session and repository-aware file opening to a renderer", () => { + const original = KANDEV_RENDERERS.list_tasks; + const renderer = vi.fn(() =>
Context probe
); + const onOpenFile = vi.fn(); + KANDEV_RENDERERS.list_tasks = renderer; + try { + renderToStaticMarkup( + , + ); + } finally { + KANDEV_RENDERERS.list_tasks = original; + } + + expect(renderer).toHaveBeenCalledWith( + expect.objectContaining({ sessionId: "session-1", onOpenFile }), + ); + }); + + it("unwraps the real Codex ACP MCP arguments before rendering rich output", () => { + const html = renderToStaticMarkup( + , + ); + + expect(html).toContain("Nested presentation"); + expect(html).not.toContain("This presentation is unavailable."); + }); + + it("renders a CSV chart from the persisted tool-result snapshot", () => { + const html = renderToStaticMarkup( + , + ); + + expect(html).toContain("CSV presentation"); + expect(html).toContain('data-testid="rich-output-chart-bar"'); + expect(html).not.toContain("This presentation is unavailable."); + }); +}); + // State labels and titles reused across multiple tool-DTOs. Pulled to // constants to satisfy the "no duplicate string" lint rule. const COMPLETED = "COMPLETED"; diff --git a/apps/web/components/task/chat/messages/kandev-tool-message.tsx b/apps/web/components/task/chat/messages/kandev-tool-message.tsx index 3b6ee68496..421a85e0d5 100644 --- a/apps/web/components/task/chat/messages/kandev-tool-message.tsx +++ b/apps/web/components/task/chat/messages/kandev-tool-message.tsx @@ -2,8 +2,8 @@ import { memo } from "react"; import type { Message } from "@/lib/types/http"; -import type { ToolCallMetadata } from "@/components/task/chat/types"; -import { extractKandevStem, extractMcpResult } from "./kandev/parse"; +import { kandevToolStemOf, type ToolCallMetadata } from "@/components/task/chat/types"; +import { extractKandevArgs, extractMcpResult } from "./kandev/parse"; import { getKandevRenderer } from "./kandev/registry"; import { PermissionActionRow } from "./permission-action-row"; import { normalizeToolCallStatus } from "./tool-status"; @@ -26,34 +26,10 @@ function derivePermissionUI( type KandevToolMessageProps = { comment: Message; permissionMessage?: Message; + sessionId?: string; + onOpenFile?: (path: string, repo?: string) => void; }; -// kandevStemOf scans the several fields that may carry the raw MCP tool name -// and returns the first one that parses to a known kandev stem. The fields -// disagree in practice: -// - `metadata.tool_name` — not set by the orchestrator today (null). -// - `metadata.title` — the raw `mcp__kandev___kandev` string. -// - `comment.content` — same raw string, redundant with title. -// - `metadata.normalized.generic.name` — the ACP adapter's *category* -// (often `"other"`) rather than the tool name, so it cannot be matched on. -// We iterate candidates instead of picking a single "preferred" one because -// the live data showed `generic.name = "other"`, which would short-circuit -// any priority-based ordering on the wrong field. -function kandevStemOf(comment: Message): string | null { - const meta = comment.metadata as ToolCallMetadata | undefined; - const candidates: Array = [ - meta?.tool_name, - meta?.title, - comment.content || undefined, - meta?.normalized?.generic?.name, - ]; - for (const candidate of candidates) { - const stem = extractKandevStem(candidate); - if (stem) return stem; - } - return null; -} - // hasKandevRenderer is the matcher used by the message dispatcher. It accepts // any `tool_call` whose tool name is recognised as a Kandev MCP tool AND for // which a per-tool renderer is registered. We require both because we still @@ -61,7 +37,7 @@ function kandevStemOf(comment: Message): string | null { // (rather than rendering an empty row) until a dedicated renderer ships. export function hasKandevRenderer(comment: Message): boolean { if (comment.type !== "tool_call") return false; - return !!getKandevRenderer(kandevStemOf(comment)); + return !!getKandevRenderer(kandevToolStemOf(comment)); } // KandevToolMessage is the rendered entry point for every Kandev tool call. @@ -73,9 +49,11 @@ export function hasKandevRenderer(comment: Message): boolean { export const KandevToolMessage = memo(function KandevToolMessage({ comment, permissionMessage, + sessionId, + onOpenFile, }: KandevToolMessageProps) { const meta = comment.metadata as ToolCallMetadata | undefined; - const renderer = getKandevRenderer(kandevStemOf(comment)); + const renderer = getKandevRenderer(kandevToolStemOf(comment)); const { permissionMetadata, permissionStatus, isPermissionPending } = parsePermission(permissionMessage); const { isResponding, handleApprove, handleAllowAlways, hasAllowAlways, handleReject } = @@ -91,8 +69,7 @@ export const KandevToolMessage = memo(function KandevToolMessage({ // component still works for any code path that hasn't routed through the // normalizer yet. const generic = meta?.normalized?.generic; - const argsCandidate = (generic?.input as Record | undefined) ?? meta?.args; - const args = argsCandidate && typeof argsCandidate === "object" ? argsCandidate : undefined; + const args = extractKandevArgs(generic?.input) ?? extractKandevArgs(meta?.args); const rawResult = generic?.output ?? meta?.result; const result = extractMcpResult(rawResult); const status = normalizeToolCallStatus(meta?.status); @@ -104,7 +81,7 @@ export const KandevToolMessage = memo(function KandevToolMessage({ // and avoids the lint rule against "components created during render". const rendered = ( - {renderer({ args, result, status })} + {renderer({ args, result, status, sessionId, onOpenFile })} ); diff --git a/apps/web/components/task/chat/messages/kandev/parse.test.ts b/apps/web/components/task/chat/messages/kandev/parse.test.ts index b2c4642b38..60eff8cf87 100644 --- a/apps/web/components/task/chat/messages/kandev/parse.test.ts +++ b/apps/web/components/task/chat/messages/kandev/parse.test.ts @@ -1,5 +1,20 @@ import { describe, expect, it } from "vitest"; -import { extractKandevStem, extractMcpResult, shortId } from "./parse"; +import { extractKandevArgs, extractKandevStem, extractMcpResult, shortId } from "./parse"; + +describe("extractKandevArgs", () => { + it("unwraps arguments from the real Codex ACP MCP envelope", () => { + const argumentsValue = { version: 1, title: "Service failures", blocks: [] }; + expect( + extractKandevArgs({ + raw_input: { + server: "kandev", + tool: "show_rich_output_kandev", + arguments: argumentsValue, + }, + }), + ).toEqual(argumentsValue); + }); +}); describe("extractKandevStem", () => { it("strips the mcp__kandev__ namespace and the _kandev suffix", () => { @@ -10,6 +25,10 @@ describe("extractKandevStem", () => { expect(extractKandevStem("kandev/list_tasks_kandev")).toBe("list_tasks"); }); + it("handles the dotted Codex ACP title fallback", () => { + expect(extractKandevStem("mcp.kandev.show_rich_output_kandev")).toBe("show_rich_output"); + }); + it("handles a bare suffix-only name", () => { expect(extractKandevStem("create_task_kandev")).toBe("create_task"); }); @@ -68,6 +87,35 @@ describe("extractMcpResult", () => { it("returns plain objects untouched", () => { expect(extractMcpResult({ foo: 1 })).toEqual({ foo: 1 }); }); + + it("prefers standard MCP structuredContent over its text fallback", () => { + expect( + extractMcpResult({ + content: [{ type: "text", text: "fallback" }], + structuredContent: { version: 1, resolved_charts: [] }, + }), + ).toEqual({ version: 1, resolved_charts: [] }); + }); + + it("unwraps the raw result wrapper emitted by ACP clients", () => { + expect(extractMcpResult({ result: '{"version":1,"resolved_charts":[]}' })).toEqual({ + version: 1, + resolved_charts: [], + }); + }); + + it("unwraps the real Codex ACP rawOutput result object", () => { + const snapshot = { version: 1, resolved_charts: [] }; + expect( + extractMcpResult({ + error: null, + result: { + content: [{ type: "text", text: JSON.stringify(snapshot) }], + structuredContent: snapshot, + }, + }), + ).toEqual(snapshot); + }); }); describe("shortId", () => { diff --git a/apps/web/components/task/chat/messages/kandev/parse.ts b/apps/web/components/task/chat/messages/kandev/parse.ts index 2001dfa1f8..a578ce2497 100644 --- a/apps/web/components/task/chat/messages/kandev/parse.ts +++ b/apps/web/components/task/chat/messages/kandev/parse.ts @@ -13,12 +13,31 @@ // usually an array of `{type, text}` content blocks where `text` is itself a // JSON-encoded string; we parse that inner JSON so the renderers see plain JS. -const NAMESPACE_SEP = /\/|__/; +const NAMESPACE_SEP = /\/|__|\./; +const KANDEV_NAMESPACES = ["mcp__kandev__", "mcp.kandev.", "kandev/"] as const; const KANDEV_SUFFIX = "_kandev"; +function asRecord(value: unknown): Record | undefined { + if (!value || typeof value !== "object" || Array.isArray(value)) return undefined; + return value as Record; +} + +// ACP stores generic tool arguments below `input.raw_input`; older/direct +// transports store the arguments at `input`. Normalize both at the renderer +// boundary so every Kandev tool sees its actual call arguments. +export function extractKandevArgs(value: unknown): Record | undefined { + const input = asRecord(value); + if (!input) return undefined; + const rawInput = asRecord(input.raw_input); + return asRecord(rawInput?.arguments) ?? rawInput ?? input; +} + export function extractKandevStem(toolName: string | undefined): string | null { if (!toolName) return null; - const tail = toolName.trim().split(NAMESPACE_SEP).pop() ?? ""; + const normalized = toolName.trim(); + const namespace = KANDEV_NAMESPACES.find((prefix) => normalized.startsWith(prefix)); + const tail = namespace ? normalized.slice(namespace.length) : normalized; + if ((!namespace && NAMESPACE_SEP.test(normalized)) || NAMESPACE_SEP.test(tail)) return null; if (!tail.endsWith(KANDEV_SUFFIX)) return null; const stem = tail.slice(0, -KANDEV_SUFFIX.length); return stem.length > 0 ? stem : null; @@ -63,9 +82,23 @@ export function extractMcpResult(value: unknown): unknown { return unwrapContentBlocks(value); } if (typeof value === "object") { - const obj = value as { content?: unknown; output?: unknown }; + const obj = value as { + content?: unknown; + output?: unknown; + result?: unknown; + structuredContent?: unknown; + structured_content?: unknown; + }; + if (obj.structuredContent !== undefined) return obj.structuredContent; + if (obj.structured_content !== undefined) return obj.structured_content; if (Array.isArray(obj.content)) return unwrapContentBlocks(obj.content); if (typeof obj.output === "string") return tryParseJson(obj.output); + if ( + obj.result !== undefined && + (Object.hasOwn(obj, "error") || Object.keys(obj).length === 1) + ) { + return extractMcpResult(obj.result); + } return value; } return value; @@ -90,10 +123,9 @@ export function pickArray(obj: unknown, key: string): T[] | undefin } export function pickObject(obj: unknown, key: string): Record | undefined { - if (!obj || typeof obj !== "object") return undefined; - const v = (obj as Record)[key]; - if (!v || typeof v !== "object" || Array.isArray(v)) return undefined; - return v as Record; + const record = asRecord(obj); + if (!record) return undefined; + return asRecord(record[key]); } // Shorten a UUID-like identifier for inline display (e.g. "abc12345…"). diff --git a/apps/web/components/task/chat/messages/kandev/registry.ts b/apps/web/components/task/chat/messages/kandev/registry.ts index 9f7f253c2a..569c0de8e1 100644 --- a/apps/web/components/task/chat/messages/kandev/registry.ts +++ b/apps/web/components/task/chat/messages/kandev/registry.ts @@ -29,6 +29,7 @@ import { } from "./document-renderers"; import { AskUserQuestionRenderer } from "./ask-user-question-renderer"; import { ShowWalkthroughRenderer } from "./walkthrough-renderer"; +import { RichOutputRenderer } from "./rich-output/rich-output-renderer"; import type { KandevRenderer } from "./types"; export const KANDEV_RENDERERS: Record = { @@ -56,6 +57,7 @@ export const KANDEV_RENDERERS: Record = { ask_user_question: AskUserQuestionRenderer, show_walkthrough: ShowWalkthroughRenderer, + show_rich_output: RichOutputRenderer, }; export function getKandevRenderer(stem: string | null): KandevRenderer | null { diff --git a/apps/web/components/task/chat/messages/kandev/rich-output/chart-block.test.tsx b/apps/web/components/task/chat/messages/kandev/rich-output/chart-block.test.tsx new file mode 100644 index 0000000000..f16ba9a767 --- /dev/null +++ b/apps/web/components/task/chat/messages/kandev/rich-output/chart-block.test.tsx @@ -0,0 +1,223 @@ +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { RichOutputChartBlock } from "./types"; + +type ObserverRecord = { + callback: IntersectionObserverCallback; + instance: IntersectionObserver; + options?: IntersectionObserverInit; + target?: Element; +}; + +const observerRecords: ObserverRecord[] = []; +const visibilityDescriptor = Object.getOwnPropertyDescriptor(document, "visibilityState"); +const MOCK_LINE_CHART = "mock-line-chart"; +const chartMotion = vi.hoisted(() => ({ enabled: true })); +const captured = vi.hoisted(() => ({ + bars: [] as Array>, + chartContainers: [] as Array>, + lineCharts: [] as Array>, + lines: [] as Array>, + xAxes: [] as Array>, + yAxes: [] as Array>, +})); + +vi.mock("./chart-motion", () => ({ + useRichOutputChartAnimations: () => chartMotion.enabled, +})); + +vi.mock("@kandev/ui/chart", () => ({ + ChartContainer: (props: { children: ReactNode }) => { + captured.chartContainers.push(props); + return
{props.children}
; + }, + ChartLegend: ({ content }: { content: ReactNode }) => <>{content}, + ChartTooltip: () => null, + ChartTooltipContent: () => null, +})); + +vi.mock("recharts", () => ({ + Bar: (props: Record) => { + captured.bars.push(props); + return null; + }, + BarChart: ({ children }: { children: ReactNode }) => ( +
{children}
+ ), + CartesianGrid: () => null, + Line: (props: Record) => { + captured.lines.push(props); + return null; + }, + LineChart: (props: { children: ReactNode }) => { + captured.lineCharts.push(props); + return
{props.children}
; + }, + XAxis: (props: Record) => { + captured.xAxes.push(props); + return null; + }, + YAxis: (props: Record) => { + captured.yAxes.push(props); + return null; + }, +})); + +import { ChartBlock } from "./chart-block"; + +const BLOCK: RichOutputChartBlock = { + type: "chart", + chart_type: "line", + title: "Latency", + summary: "Latency over time", + labels: ["Mon", "Tue"], + series: [{ label: "p95", values: [120, 110] }], +}; + +function installIntersectionObserver() { + class MockIntersectionObserver implements IntersectionObserver { + readonly root = null; + readonly rootMargin = "0px"; + readonly thresholds: readonly number[] = []; + private readonly record: ObserverRecord; + + constructor(callback: IntersectionObserverCallback, options?: IntersectionObserverInit) { + this.record = { callback, instance: this, options }; + observerRecords.push(this.record); + } + + disconnect() {} + observe(target: Element) { + this.record.target = target; + } + takeRecords(): IntersectionObserverEntry[] { + return []; + } + unobserve() {} + } + + vi.stubGlobal("IntersectionObserver", MockIntersectionObserver); +} + +function intersect(record = observerRecords[0]) { + act(() => { + record.callback( + [{ isIntersecting: true, target: record.target } as IntersectionObserverEntry], + record.instance, + ); + }); +} + +function setDocumentVisibility(value: DocumentVisibilityState) { + Object.defineProperty(document, "visibilityState", { configurable: true, value }); + document.dispatchEvent(new Event("visibilitychange")); +} + +beforeEach(() => { + observerRecords.length = 0; + for (const values of Object.values(captured)) values.length = 0; + installIntersectionObserver(); + chartMotion.enabled = true; +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + if (visibilityDescriptor) { + Object.defineProperty(document, "visibilityState", visibilityDescriptor); + } else { + Reflect.deleteProperty(document, "visibilityState"); + } +}); + +describe("ChartBlock plot scheduling", () => { + it("defers the Recharts plot until the chart approaches the viewport", () => { + render(); + + expect(screen.queryByTestId(MOCK_LINE_CHART)).toBeNull(); + expect(screen.getByText(BLOCK.title)).not.toBeNull(); + expect(observerRecords[0].options?.rootMargin).toBe("200px 0px"); + + intersect(); + expect(screen.getByTestId(MOCK_LINE_CHART)).not.toBeNull(); + }); + + it("waits for a background tab to become visible after intersection", () => { + setDocumentVisibility("hidden"); + render(); + + intersect(); + expect(screen.queryByTestId(MOCK_LINE_CHART)).toBeNull(); + + act(() => setDocumentVisibility("visible")); + expect(screen.getByTestId(MOCK_LINE_CHART)).not.toBeNull(); + }); + + it("keeps an eligible plot mounted after it leaves the visible tab", () => { + render(); + intersect(); + const mountedPlot = screen.getByTestId(MOCK_LINE_CHART); + + act(() => setDocumentVisibility("hidden")); + + expect(screen.getByTestId(MOCK_LINE_CHART)).toBe(mountedPlot); + }); + + it("renders immediately when intersection observation is unavailable", () => { + vi.stubGlobal("IntersectionObserver", undefined); + + render(); + + expect(screen.getByTestId(MOCK_LINE_CHART)).not.toBeNull(); + }); + + it("reuses derived chart inputs when a legend toggle changes local visibility", () => { + const block: RichOutputChartBlock = { + ...BLOCK, + series: [BLOCK.series[0], { label: "p50", values: [80, 75] }], + }; + render(); + intersect(); + + const initialData = captured.lineCharts.at(-1)?.data; + const initialConfig = captured.chartContainers.at(-1)?.config; + const initialXAxisFormatter = captured.xAxes.at(-1)?.tickFormatter; + const initialYAxisFormatter = captured.yAxes.at(-1)?.tickFormatter; + + fireEvent.click(screen.getByRole("button", { name: "p50" })); + + expect(captured.lineCharts.at(-1)?.data).toBe(initialData); + expect(captured.chartContainers.at(-1)?.config).toBe(initialConfig); + expect(captured.xAxes.at(-1)?.tickFormatter).toBe(initialXAxisFormatter); + expect(captured.yAxes.at(-1)?.tickFormatter).toBe(initialYAxisFormatter); + expect(captured.lines.at(-1)?.hide).toBe(true); + }); + + it("skips an unchanged parent rerender and retains default chart animation", () => { + const view = render(); + intersect(); + const renderCount = captured.lineCharts.length; + + view.rerender(); + + expect(captured.lineCharts).toHaveLength(renderCount); + expect(captured.lines.at(-1)?.isAnimationActive).not.toBe(false); + }); + + it("disables every line and bar animation when effective chart motion is off", () => { + chartMotion.enabled = false; + const line = render(); + intersect(); + + expect(captured.lines.at(-1)?.isAnimationActive).toBe(false); + + line.unmount(); + observerRecords.length = 0; + const barBlock: RichOutputChartBlock = { ...BLOCK, chart_type: "bar" }; + render(); + intersect(); + + expect(captured.bars.at(-1)?.isAnimationActive).toBe(false); + }); +}); diff --git a/apps/web/components/task/chat/messages/kandev/rich-output/chart-block.tsx b/apps/web/components/task/chat/messages/kandev/rich-output/chart-block.tsx new file mode 100644 index 0000000000..d45973365d --- /dev/null +++ b/apps/web/components/task/chat/messages/kandev/rich-output/chart-block.tsx @@ -0,0 +1,232 @@ +"use client"; + +import { memo, useCallback, useMemo, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Bar, BarChart, CartesianGrid, Line, LineChart, XAxis, YAxis } from "recharts"; +import { + ChartContainer, + ChartLegend, + ChartTooltip, + ChartTooltipContent, + type ChartConfig, +} from "@kandev/ui/chart"; +import { createXAxisTickFormatter, formatYAxisTick } from "./chart-format"; +import { useRichOutputChartAnimations } from "./chart-motion"; +import { useChartPlotVisibility } from "./chart-visibility"; +import type { RichOutputChartBlock } from "./types"; + +const HOST_SERIES_COLORS = [ + "var(--chart-1)", + "var(--chart-2)", + "var(--chart-3)", + "var(--chart-4)", +] as const; +const CHART_MARGIN = { left: 0, right: 8 } as const; +const BAR_RADIUS: [number, number, number, number] = [3, 3, 0, 0]; + +function seriesKey(index: number): string { + return `series_${index}`; +} + +function chartConfig(block: RichOutputChartBlock): ChartConfig { + return Object.fromEntries( + block.series.map((series, index) => [ + seriesKey(index), + { label: series.label, color: HOST_SERIES_COLORS[index] }, + ]), + ); +} + +function chartData(block: RichOutputChartBlock): Array> { + return block.labels.map((label, labelIndex) => { + const row: Record = { label }; + for (let seriesIndex = 0; seriesIndex < block.series.length; seriesIndex += 1) { + row[seriesKey(seriesIndex)] = block.series[seriesIndex].values[labelIndex]; + } + return row; + }); +} + +function SeriesMarker({ color }: { color: string }) { + return ( +