Improve tool detail formatting

internal/telegram/bot.go

@@ -1130,7 +1130,7 @@ func (b *Bot) handleApprovalCallback(ctx context.Context, callback *CallbackQuer
 		if err := b.tg.AnswerCallbackQuery(ctx, callback.ID, "Details sent."); err != nil {
 			return err
 		}
-		return b.sendLong(ctx, callback.Message.Chat.ID, approval.PayloadJSON)
+		return b.sendHTML(ctx, callback.Message.Chat.ID, renderApprovalHTML(approval.Kind, json.RawMessage(approval.PayloadJSON), ""))
 	}
 	if approval.Status != "pending" {
 		return b.tg.AnswerCallbackQuery(ctx, callback.ID, "Already resolved.")
@@ -1194,7 +1194,7 @@ func parseCodexThreadItem(raw json.RawMessage) (codexThreadItemView, error) {
 func renderCodexItemStarted(item codexThreadItemView) string {
 	switch item.Type {
 	case "commandExecution":
-		return SummaryDetailsHTMLLimited(joinNonEmpty("Tool call: command started", commandSummaryLine(item.Command)), commandStartedDetails(item), TelegramHTMLMessageLimit)
+		return SummaryDetailsRawHTMLLimited(joinNonEmpty("Tool call: command started", commandSummaryLine(item.Command)), commandStartedDetailsHTML(item), TelegramHTMLMessageLimit)
 	case "fileChange":
 		return "Tool call: file change started"
 	case "mcpToolCall":
@@ -1221,17 +1221,19 @@ func renderCodexItemCompleted(item codexThreadItemView) string {
 		if item.ExitCode != nil {
 			status = fmt.Sprintf("Exit code: %d", *item.ExitCode)
 		}
-		return SummaryDetailsHTMLLimited(joinNonEmpty("Tool call: command finished", commandSummaryLine(item.Command), status), renderCodexItemDetails(item), TelegramHTMLMessageLimit)
+		return SummaryDetailsRawHTMLLimited(joinNonEmpty("Tool call: command finished", commandSummaryLine(item.Command), status), renderCodexItemDetailsHTML(item), TelegramHTMLMessageLimit)
 	case "fileChange":
 		return joinNonEmpty("Tool call: file change finished", fmt.Sprintf("Changed files: %d", len(item.Changes)), "Status: "+item.Status)
 	case "mcpToolCall":
-		return joinNonEmpty("Tool call: MCP finished", "Tool: "+toolDisplayName(item.Server, item.Tool), "Status: "+item.Status)
+		summary := joinNonEmpty("Tool call: MCP finished", "Tool: "+toolDisplayName(item.Server, item.Tool), "Status: "+item.Status)
+		return SummaryDetailsRawHTMLLimited(summary, renderCodexItemDetailsHTML(item), TelegramHTMLMessageLimit)
 	case "dynamicToolCall":
 		status := item.Status
 		if item.Success != nil {
 			status = fmt.Sprintf("success=%t", *item.Success)
 		}
-		return joinNonEmpty("Tool call: finished", "Tool: "+toolDisplayName(item.Namespace, item.Tool), "Status: "+status)
+		summary := joinNonEmpty("Tool call: finished", "Tool: "+toolDisplayName(item.Namespace, item.Tool), "Status: "+status)
+		return SummaryDetailsRawHTMLLimited(summary, renderCodexItemDetailsHTML(item), TelegramHTMLMessageLimit)
 	case "webSearch":
 		return joinNonEmpty("Tool call: web search finished", "Query: "+item.Query)
 	case "imageView":
@@ -1257,66 +1259,245 @@ func commandSummaryLine(command string) string {
 	return "Command: " + string(runes[:commandSummaryLimit]) + "..."
 }
 
-func commandStartedDetails(item codexThreadItemView) string {
-	var lines []string
-	if strings.TrimSpace(item.Command) != "" && len([]rune(strings.TrimSpace(item.Command))) > commandSummaryLimit {
-		lines = append(lines, "command: "+strings.TrimSpace(item.Command))
+func commandStartedDetailsHTML(item codexThreadItemView) string {
+	var parts []string
+	if command := strings.TrimSpace(item.Command); command != "" && len([]rune(command)) > commandSummaryLimit {
+		parts = append(parts, "<b>Command</b>", CodeBlockHTML("bash", command))
 	}
-	if strings.TrimSpace(item.CWD) != "" {
-		lines = append(lines, "cwd: "+strings.TrimSpace(item.CWD))
+	if cwd := strings.TrimSpace(item.CWD); cwd != "" {
+		parts = append(parts, FieldHTML("CWD", cwd))
 	}
-	return strings.Join(lines, "\n")
+	return strings.Join(parts, "\n")
 }
 
-func renderCodexItemDetails(item codexThreadItemView) string {
-	var lines []string
-	appendKV := func(key string, value any) {
+func renderCodexItemDetailsHTML(item codexThreadItemView) string {
+	var parts []string
+	appendField := func(label, value string) {
+		if html := FieldHTML(label, value); html != "" {
+			parts = append(parts, html)
+		}
+	}
+	appendInt := func(label string, value *int) {
+		if value != nil {
+			appendField(label, strconv.Itoa(*value))
+		}
+	}
+	appendInt64 := func(label string, value *int64) {
+		if value != nil {
+			appendField(label, strconv.FormatInt(*value, 10))
+		}
+	}
+	appendBool := func(label string, value *bool) {
+		if value != nil {
+			appendField(label, strconv.FormatBool(*value))
+		}
+	}
+
+	switch item.Type {
+	case "commandExecution":
+		appendField("CWD", item.CWD)
+		if command := strings.TrimSpace(item.Command); command != "" && len([]rune(command)) > commandSummaryLimit {
+			parts = append(parts, "<b>Command</b>", CodeBlockHTML("bash", command))
+		}
+		appendInt("Exit code", item.ExitCode)
+		appendInt64("Duration ms", item.DurationMs)
+		if item.AggregatedOutput != nil && strings.TrimSpace(*item.AggregatedOutput) != "" {
+			parts = append(parts, "<b>Output</b>", CodeBlockHTML("text", *item.AggregatedOutput))
+		}
+	case "fileChange":
+		appendField("Status", item.Status)
+		for _, change := range item.Changes {
+			appendField("Changed", change.Path)
+		}
+	case "mcpToolCall":
+		appendField("Tool", toolDisplayName(item.Server, item.Tool))
+		appendField("Status", item.Status)
+		parts = append(parts, renderArgumentsDetailsHTML(item.Arguments)...)
+	case "dynamicToolCall":
+		appendField("Tool", toolDisplayName(item.Namespace, item.Tool))
+		appendField("Status", item.Status)
+		appendBool("Success", item.Success)
+		parts = append(parts, renderArgumentsDetailsHTML(item.Arguments)...)
+	case "webSearch":
+		appendField("Query", item.Query)
+	case "imageView":
+		appendField("Path", item.Path)
+	case "imageGeneration":
+		appendField("Status", item.Status)
+		appendField("Saved path", item.SavedPath)
+	case "collabAgentToolCall":
+		appendField("Tool", item.Tool)
+		appendField("Status", item.Status)
+	default:
+		appendField("Type", item.Type)
+		appendField("Status", item.Status)
+		parts = append(parts, renderArgumentsDetailsHTML(item.Arguments)...)
+	}
+	return strings.Join(nonEmptyHTML(parts), "\n")
+}
+
+func renderArgumentsDetailsHTML(raw json.RawMessage) []string {
+	if len(raw) == 0 || string(raw) == "null" {
+		return nil
+	}
+	var value any
+	if err := json.Unmarshal(raw, &value); err != nil {
+		return []string{"<b>Arguments</b>", CodeBlockHTML("json", string(raw))}
+	}
+	object, ok := value.(map[string]any)
+	if !ok {
+		return []string{"<b>Arguments</b>", CodeBlockHTML("json", compactPrettyJSON(value))}
+	}
+
+	var parts []string
+	for _, key := range preferredArgumentKeys(object) {
+		part := renderArgumentFieldHTML(key, object[key])
+		if strings.TrimSpace(part) != "" {
+			parts = append(parts, part)
+		}
+	}
+	if len(parts) == 0 && len(object) > 0 {
+		parts = append(parts, "<b>Arguments</b>", CodeBlockHTML("json", compactPrettyJSON(object)))
+	}
+	return parts
+}
+
+func preferredArgumentKeys(object map[string]any) []string {
+	preferred := []string{"cmd", "command", "script", "code", "content", "input", "query", "path", "file", "filename", "cwd", "args", "config", "patch"}
+	seen := make(map[string]bool, len(object))
+	var keys []string
+	for _, key := range preferred {
+		if _, ok := object[key]; ok {
+			keys = append(keys, key)
+			seen[key] = true
+		}
+	}
+	for key := range object {
+		if !seen[key] && isMeaningfulArgumentKey(key) {
+			keys = append(keys, key)
+		}
+	}
+	return keys
+}
+
+func isMeaningfulArgumentKey(key string) bool {
+	key = strings.ToLower(key)
+	for _, part := range []string{"command", "cmd", "code", "content", "path", "file", "query", "input", "config", "patch", "cwd"} {
+		if strings.Contains(key, part) {
+			return true
+		}
+	}
+	return false
+}
+
+func renderArgumentFieldHTML(key string, value any) string {
+	label := argumentLabel(key)
+	text, complex := argumentValueText(value)
+	if strings.TrimSpace(text) == "" {
+		return ""
+	}
+	if complex || shouldUseCodeBlock(key, text) {
+		return "<b>" + EscapeHTML(label) + "</b>\n" + CodeBlockHTML(languageForArgument(key, text), text)
+	}
+	return FieldHTML(label, text)
+}
+
+func argumentLabel(key string) string {
+	key = strings.TrimSpace(key)
+	if key == "" {
+		return "Argument"
+	}
+	switch strings.ToLower(key) {
+	case "cwd":
+		return "CWD"
+	case "cmd":
+		return "cmd"
+	}
+	label := strings.ReplaceAll(key, "_", " ")
+	return strings.ToUpper(label[:1]) + label[1:]
+}
+
+func argumentValueText(value any) (string, bool) {
 	switch v := value.(type) {
 	case string:
-			if strings.TrimSpace(v) != "" {
-				lines = append(lines, fmt.Sprintf("%s: %s", key, v))
+		return v, false
+	case float64:
+		return strconv.FormatFloat(v, 'f', -1, 64), false
+	case bool:
+		return strconv.FormatBool(v), false
+	case nil:
+		return "", false
+	default:
+		return compactPrettyJSON(v), true
 	}
-		case *int:
-			if v != nil {
-				lines = append(lines, fmt.Sprintf("%s: %d", key, *v))
+}
+
+func shouldUseCodeBlock(key, text string) bool {
+	key = strings.ToLower(key)
+	if strings.Contains(text, "\n") {
+		return true
 	}
-		case *int64:
-			if v != nil {
-				lines = append(lines, fmt.Sprintf("%s: %d", key, *v))
-			}
-		case *bool:
-			if v != nil {
-				lines = append(lines, fmt.Sprintf("%s: %t", key, *v))
+	for _, marker := range []string{"command", "cmd", "script", "code", "content", "config", "patch"} {
+		if strings.Contains(key, marker) {
+			return true
 		}
 	}
+	return false
+}
+
+func languageForArgument(key, text string) string {
+	key = strings.ToLower(key)
+	switch {
+	case strings.Contains(key, "command") || strings.Contains(key, "cmd") || strings.Contains(key, "script"):
+		return "bash"
+	case strings.Contains(key, "config") || looksLikeJSON(text):
+		return "json"
+	case strings.Contains(key, "patch"):
+		return "diff"
+	case strings.Contains(key, "code") || strings.Contains(key, "content"):
+		return languageForContent(text)
+	default:
+		return "text"
 	}
-	appendKV("type", item.Type)
-	appendKV("id", item.ID)
-	appendKV("command", item.Command)
-	appendKV("cwd", item.CWD)
-	appendKV("status", item.Status)
-	appendKV("tool", toolDisplayName(item.Namespace, item.Tool))
-	appendKV("server", item.Server)
-	appendKV("query", item.Query)
-	appendKV("path", item.Path)
-	appendKV("savedPath", item.SavedPath)
-	appendKV("exitCode", item.ExitCode)
-	appendKV("durationMs", item.DurationMs)
-	appendKV("success", item.Success)
-	if len(item.Arguments) > 0 && string(item.Arguments) != "null" {
-		lines = append(lines, "arguments: "+string(item.Arguments))
+}
+
+func languageForContent(text string) string {
+	trimmed := strings.TrimSpace(text)
+	switch {
+	case looksLikeJSON(trimmed):
+		return "json"
+	case strings.HasPrefix(trimmed, "package ") || strings.Contains(trimmed, "func "):
+		return "go"
+	case strings.Contains(trimmed, "#!/bin/sh") || strings.Contains(trimmed, "#!/usr/bin/env bash"):
+		return "bash"
+	case strings.Contains(trimmed, "apiVersion:") || strings.Contains(trimmed, "kind:"):
+		return "yaml"
+	default:
+		return "text"
 	}
-	if len(item.Changes) > 0 {
-		for _, change := range item.Changes {
-			if change.Path != "" {
-				lines = append(lines, "changed: "+change.Path)
+}
+
+func looksLikeJSON(text string) bool {
+	text = strings.TrimSpace(text)
+	return (strings.HasPrefix(text, "{") && strings.HasSuffix(text, "}")) || (strings.HasPrefix(text, "[") && strings.HasSuffix(text, "]"))
+}
+
+func compactPrettyJSON(value any) string {
+	data, err := json.MarshalIndent(value, "", "  ")
+	if err != nil {
+		return fmt.Sprint(value)
+	}
+	return string(data)
+}
+
+func nonEmptyHTML(parts []string) []string {
+	out := make([]string, 0, len(parts))
+	for _, part := range parts {
+		if strings.TrimSpace(part) != "" {
+			out = append(out, part)
 		}
 	}
-	}
-	if item.AggregatedOutput != nil && strings.TrimSpace(*item.AggregatedOutput) != "" {
-		lines = append(lines, "output:\n"+strings.TrimSpace(*item.AggregatedOutput))
-	}
-	return strings.Join(lines, "\n")
+	return out
 }
 
 func joinNonEmpty(lines ...string) string {
@@ -2543,23 +2724,57 @@ func renderApprovalHTML(kind string, raw json.RawMessage, status string) string
 	}
 	for _, key := range []string{"command", "cwd", "grantRoot", "permissions"} {
 		if value, ok := params[key]; ok {
-			lines = append(lines, fmt.Sprintf("%s: %s", key, conciseValue(value)))
+			lines = append(lines, fmt.Sprintf("%s: %s", argumentLabel(key), conciseValue(value)))
 		}
 	}
 
 	summary := strings.Join(lines, "\n")
-	details := prettyJSON(raw)
+	details := renderApprovalDetailsHTML(kind, raw)
 	limit := TelegramHTMLMessageLimit
 	if status != "" {
 		limit -= len([]rune(status)) + 1
 	}
-	text := SummaryDetailsHTMLLimited(summary, details, limit)
+	text := SummaryDetailsRawHTMLLimited(summary, details, limit)
 	if status != "" {
 		text += "\n" + EscapeHTML(status)
 	}
 	return text
 }
 
+func renderApprovalDetailsHTML(kind string, raw json.RawMessage) string {
+	var params map[string]any
+	if err := json.Unmarshal(raw, &params); err != nil {
+		return CodeBlockHTML("json", string(raw))
+	}
+	var parts []string
+	appendValue := func(label string, value any) {
+		text, complex := argumentValueText(value)
+		if strings.TrimSpace(text) == "" {
+			return
+		}
+		if complex || strings.Contains(text, "\n") || strings.EqualFold(label, "Command") || strings.EqualFold(label, "Permissions") {
+			language := "text"
+			if strings.EqualFold(label, "Command") {
+				language = "bash"
+			} else if complex || strings.EqualFold(label, "Permissions") || looksLikeJSON(text) {
+				language = "json"
+			}
+			parts = append(parts, "<b>"+EscapeHTML(label)+"</b>", CodeBlockHTML(language, text))
+			return
+		}
+		parts = append(parts, FieldHTML(label, text))
+	}
+	for _, key := range []string{"command", "cwd", "grantRoot", "permissions", "reason"} {
+		if value, ok := params[key]; ok {
+			appendValue(argumentLabel(key), value)
+		}
+	}
+	if len(parts) == 0 {
+		return CodeBlockHTML("json", prettyJSON(raw))
+	}
+	return strings.Join(nonEmptyHTML(parts), "\n")
+}
+
 func approvalStatusLine(decision string) string {
 	switch decision {
 	case "accept":
@@ -2577,7 +2792,9 @@ func approvalStatusLine(decision string) string {
 
 func conciseValue(value any) string {
 	text := fmt.Sprint(value)
-	if data, err := json.Marshal(value); err == nil {
+	if stringValue, ok := value.(string); ok {
+		text = stringValue
+	} else if data, err := json.Marshal(value); err == nil {
 		text = string(data)
 	}
 	text = strings.Join(strings.Fields(text), " ")

internal/telegram/render.go

@@ -35,6 +35,119 @@ func ExpandableQuoteHTML(text string) string {
 	return "<blockquote expandable>" + EscapeHTML(text) + "</blockquote>"
 }
 
+func ExpandableQuoteRawHTML(htmlText string) string {
+	htmlText = strings.TrimSpace(htmlText)
+	if htmlText == "" {
+		return ""
+	}
+	return "<blockquote expandable>" + htmlText + "</blockquote>"
+}
+
+func SummaryDetailsRawHTML(summary, detailsHTML string) string {
+	summary = strings.TrimSpace(summary)
+	detailsHTML = strings.TrimSpace(detailsHTML)
+	if detailsHTML == "" {
+		return EscapeHTML(summary)
+	}
+	if summary == "" {
+		return ExpandableQuoteRawHTML(detailsHTML)
+	}
+	return EscapeHTML(summary) + "\n" + ExpandableQuoteRawHTML(detailsHTML)
+}
+
+func SummaryDetailsRawHTMLLimited(summary, detailsHTML string, limit int) string {
+	if limit <= 0 {
+		limit = TelegramHTMLMessageLimit
+	}
+	summary = strings.TrimSpace(summary)
+	detailsHTML = strings.TrimSpace(detailsHTML)
+	out := SummaryDetailsRawHTML(summary, detailsHTML)
+	if len([]rune(out)) <= limit || detailsHTML == "" {
+		return out
+	}
+
+	plain := stripSimpleHTML(detailsHTML)
+	suffix := "\n...[truncated]"
+	runes := []rune(plain)
+	for len(runes) > 0 {
+		candidateLen := len(runes) - max(1, (len([]rune(out))-limit)/2)
+		if candidateLen < 0 {
+			candidateLen = 0
+		}
+		if candidateLen > len(runes) {
+			candidateLen = len(runes)
+		}
+		candidate := CodeBlockHTML("text", strings.TrimSpace(string(runes[:candidateLen]))+suffix)
+		out = SummaryDetailsRawHTML(summary, candidate)
+		if len([]rune(out)) <= limit || candidateLen == 0 {
+			return out
+		}
+		runes = runes[:candidateLen]
+	}
+	return SummaryDetailsRawHTML(summary, EscapeHTML(suffix))
+}
+
+func CodeBlockHTML(language, text string) string {
+	text = strings.TrimSpace(text)
+	if text == "" {
+		return ""
+	}
+	language = safeCodeLanguage(language)
+	return "<pre><code class=\"language-" + language + "\">" + EscapeHTML(text) + "</code></pre>"
+}
+
+func FieldHTML(label, value string) string {
+	value = strings.TrimSpace(value)
+	if value == "" {
+		return ""
+	}
+	return "<b>" + EscapeHTML(label) + ":</b> " + EscapeHTML(value)
+}
+
+func safeCodeLanguage(language string) string {
+	language = strings.ToLower(strings.TrimSpace(language))
+	if language == "" {
+		return "text"
+	}
+	var builder strings.Builder
+	for _, r := range language {
+		switch {
+		case r >= 'a' && r <= 'z':
+			builder.WriteRune(r)
+		case r >= '0' && r <= '9':
+			builder.WriteRune(r)
+		case r == '-', r == '_':
+			builder.WriteRune(r)
+		}
+	}
+	if builder.Len() == 0 {
+		return "text"
+	}
+	return builder.String()
+}
+
+func stripSimpleHTML(htmlText string) string {
+	replacer := strings.NewReplacer(
+		"<pre>", "", "</pre>", "",
+		"<code>", "", "</code>", "",
+		"<b>", "", "</b>", "",
+		"<i>", "", "</i>", "",
+	)
+	text := replacer.Replace(htmlText)
+	for {
+		start := strings.Index(text, "<code ")
+		if start < 0 {
+			break
+		}
+		end := strings.Index(text[start:], ">")
+		if end < 0 {
+			break
+		}
+		text = text[:start] + text[start+end+1:]
+	}
+	return html.UnescapeString(text)
+}
+
 func SummaryDetailsHTMLLimited(summary, details string, limit int) string {
 	if limit <= 0 {
 		limit = TelegramHTMLMessageLimit

internal/telegram/render_test.go

@@ -1,6 +1,7 @@
 package telegram
 
 import (
+	"encoding/json"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -155,7 +156,7 @@ func TestRenderCodexCommandExecutionItem(t *testing.T) {
 		ExitCode:         &exitCode,
 	}
 	text := renderCodexItemCompleted(item)
-	for _, want := range []string{"Tool call: command finished", "Command: go test ./...", "Exit code: 0", "line 1"} {
+	for _, want := range []string{"Tool call: command finished", "Command: go test ./...", "Exit code: 0", "<pre><code class=\"language-text\">line 1\nline 2</code></pre>"} {
 		if !strings.Contains(text, want) {
 			t.Fatalf("rendered command item missing %q in %q", want, text)
 		}
@@ -169,6 +170,38 @@ func TestRenderCodexStartedItems(t *testing.T) {
 	}
 }
 
+func TestRenderDynamicToolDetailsSelectsUsefulArguments(t *testing.T) {
+	item := codexThreadItemView{
+		Type:      "dynamicToolCall",
+		Namespace: "functions",
+		Tool:      "exec_command",
+		Status:    "completed",
+		Arguments: json.RawMessage(`{"cmd":"go test ./...","irrelevant":{"large":"object"}}`),
+	}
+	text := renderCodexItemCompleted(item)
+	for _, want := range []string{"Tool: functions.exec_command", "<b>cmd</b>", "language-bash", "go test ./..."} {
+		if !strings.Contains(text, want) {
+			t.Fatalf("rendered tool details missing %q in %q", want, text)
+		}
+	}
+	if strings.Contains(text, "irrelevant") {
+		t.Fatalf("rendered tool details should omit irrelevant argument JSON: %q", text)
+	}
+}
+
+func TestRenderApprovalDetailsAvoidsRawJSONDump(t *testing.T) {
+	raw := json.RawMessage(`{"command":"go test ./...","cwd":"/workspace/project","unused":{"nested":true}}`)
+	text := renderApprovalHTML("item/commandExecution/requestApproval", raw, "")
+	for _, want := range []string{"Codex requests command approval", "language-bash", "go test ./...", "CWD"} {
+		if !strings.Contains(text, want) {
+			t.Fatalf("approval render missing %q in %q", want, text)
+		}
+	}
+	if strings.Contains(text, "unused") {
+		t.Fatalf("approval render should omit unused JSON: %q", text)
+	}
+}
+
 func TestToolMessageAddsEditedAtBeforeDetails(t *testing.T) {
 	tool := toolMessageState{
 		toolHTML: SummaryDetailsHTML("Tool call: command finished\nCommand: go test ./...", "full output"),