Unify approval tool message rendering

internal/telegram/bot.go

@@ -1285,10 +1285,10 @@ func renderCodexItemDetailsHTML(item codexThreadItemView) string {
 
 	switch item.Type {
 	case "commandExecution":
-		appendField("CWD", item.CWD)
 		if command := strings.TrimSpace(item.Command); command != "" {
 			parts = append(parts, "<b>Command</b>", CodeBlockHTML("bash", command))
 		}
+		appendField("CWD", item.CWD)
 		appendInt("Exit code", item.ExitCode)
 		appendInt64("Duration ms", item.DurationMs)
 		if item.AggregatedOutput != nil && strings.TrimSpace(*item.AggregatedOutput) != "" {
@@ -1339,16 +1339,21 @@ func renderArgumentsDetailsHTML(raw json.RawMessage) []string {
 		return []string{"<b>Arguments</b>", CodeBlockHTML("json", compactPrettyJSON(value))}
 	}
 
+	parts := renderSelectedArgumentDetailsHTML(object, preferredArgumentKeys(object))
+	if len(parts) == 0 && len(object) > 0 {
+		parts = append(parts, "<b>Arguments</b>", CodeBlockHTML("json", compactPrettyJSON(object)))
+	}
+	return parts
+}
+
+func renderSelectedArgumentDetailsHTML(object map[string]any, keys []string) []string {
 	var parts []string
-	for _, key := range preferredArgumentKeys(object) {
+	for _, key := range keys {
 		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
 }
 
@@ -1764,16 +1769,7 @@ func (b *Bot) handleCodexServerRequest(ctx context.Context, event codexapp.Event
 	}
 	text := renderApprovalHTML(kind, event.Params, "")
 	markup := approvalMarkup(approval.ID)
-	if msg, ok, err := b.attachApprovalToToolMessage(ctx, threadID, itemID, text, markup); err != nil {
+	msg, err := b.upsertApprovalMessage(ctx, thread.TelegramUserID, threadID, itemID, text, markup)
-		return err
-	} else if ok {
-		return b.store.UpdatePendingApprovalMessage(ctx, approval.ID, msg.Chat.ID, msg.MessageID)
-	}
-
-	msg, err := b.tg.SendMessage(ctx, thread.TelegramUserID, text, SendMessageOptions{
-		ParseMode:   "HTML",
-		ReplyMarkup: markup,
-	})
 	if err != nil {
 		return err
 	}
@@ -2111,26 +2107,30 @@ func (b *Bot) upsertToolMessage(ctx context.Context, threadID, itemID, htmlText
 	return nil
 }
 
-func (b *Bot) attachApprovalToToolMessage(ctx context.Context, threadID, itemID, approvalHTML string, markup *InlineKeyboardMarkup) (Message, bool, error) {
+func (b *Bot) upsertApprovalMessage(ctx context.Context, chatID int64, threadID, itemID, approvalHTML string, markup *InlineKeyboardMarkup) (Message, error) {
 	approvalHTML = strings.TrimSpace(approvalHTML)
-	if threadID == "" || itemID == "" || approvalHTML == "" {
+	if approvalHTML == "" {
-		return Message{}, false, nil
+		return Message{}, errors.New("approval message is empty")
+	}
+	if threadID == "" || itemID == "" {
+		return b.tg.SendMessage(ctx, chatID, approvalHTML, SendMessageOptions{ParseMode: "HTML", ReplyMarkup: markup})
 	}
 	if err := b.flushAssistantMessage(ctx, threadID); err != nil {
-		return Message{}, false, err
+		return Message{}, err
+	}
+	chatID, err := b.outputChatID(ctx, threadID)
+	if err != nil {
+		return Message{}, err
 	}
 
 	b.mu.Lock()
 	state := b.outputs[threadID]
-	if state == nil {
+	if state != nil && state.tools == nil {
-		b.mu.Unlock()
+		state.tools = make(map[string]toolMessageState)
-		return Message{}, false, nil
 	}
+	if state != nil {
 		tool, ok := state.tools[itemID]
-	if !ok || tool.messageID == 0 {
+		if ok && tool.messageID != 0 {
-		b.mu.Unlock()
-		return Message{}, false, nil
-	}
 			tool.approvalHTML = approvalHTML
 			tool.approvalMarkup = markup
 			tool.editedAt = editedAtTimestamp()
@@ -2138,15 +2138,33 @@ func (b *Bot) attachApprovalToToolMessage(ctx context.Context, threadID, itemID,
 			combined := tool.html()
 			msg := Message{MessageID: tool.messageID, Chat: Chat{ID: tool.chatID}}
 			b.mu.Unlock()
-
 			_, err := b.tg.EditMessageText(ctx, msg.Chat.ID, msg.MessageID, combined, EditMessageTextOptions{ParseMode: "HTML", ReplyMarkup: editReplyMarkup(markup)})
 			if err := ignoreTelegramMessageNotModified(err); err != nil {
 				b.clearToolApproval(threadID, itemID)
 				b.logger.Printf("edit tool approval message %s/%s: %v", threadID, itemID, err)
-		return Message{}, false, nil
+				return Message{}, err
 			}
 			b.markOutputSent(threadID)
-	return msg, true, nil
+			return msg, nil
+		}
+	}
+	b.mu.Unlock()
+
+	msg, err := b.sendHTMLMessage(ctx, chatID, approvalHTML, markup)
+	if err != nil {
+		return Message{}, err
+	}
+	b.mu.Lock()
+	state = b.outputs[threadID]
+	if state != nil {
+		if state.tools == nil {
+			state.tools = make(map[string]toolMessageState)
+		}
+		state.tools[itemID] = toolMessageState{chatID: msg.Chat.ID, messageID: msg.MessageID, approvalHTML: approvalHTML, approvalMarkup: markup}
+	}
+	b.mu.Unlock()
+	b.markOutputSent(threadID)
+	return msg, nil
 }
 
 func (b *Bot) clearToolApproval(threadID, itemID string) {
@@ -2808,12 +2826,6 @@ func renderApprovalHTML(kind string, raw json.RawMessage, status string) string
 	if reason, _ := params["reason"].(string); reason != "" {
 		lines = append(lines, "", reason)
 	}
-	for _, key := range []string{"command", "cwd", "grantRoot", "permissions", "fileChanges"} {
-		if value, ok := params[key]; ok {
-			lines = append(lines, fmt.Sprintf("%s: %s", argumentLabel(key), conciseValue(value)))
-		}
-	}
-
 	summary := strings.Join(lines, "\n")
 	details := renderApprovalDetailsHTML(kind, raw)
 	limit := TelegramHTMLMessageLimit
@@ -2832,29 +2844,7 @@ func renderApprovalDetailsHTML(kind string, raw json.RawMessage) string {
 	if err := json.Unmarshal(raw, &params); err != nil {
 		return CodeBlockHTML("json", string(raw))
 	}
-	var parts []string
+	parts := renderSelectedArgumentDetailsHTML(params, []string{"command", "cwd", "grantRoot", "permissions", "fileChanges", "parsedCmd", "reason"})
-	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", "fileChanges", "parsedCmd", "reason"} {
-		if value, ok := params[key]; ok {
-			appendValue(argumentLabel(key), value)
-		}
-	}
 	if len(parts) == 0 {
 		return CodeBlockHTML("json", prettyJSON(raw))
 	}
@@ -2876,21 +2866,6 @@ func approvalStatusLine(decision string) string {
 	}
 }
 
-func conciseValue(value any) string {
-	text := fmt.Sprint(value)
-	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), " ")
-	runes := []rune(text)
-	if len(runes) > 180 {
-		return string(runes[:180]) + "..."
-	}
-	return text
-}
-
 func prettyJSON(raw json.RawMessage) string {
 	if len(raw) == 0 {
 		return ""

internal/telegram/render_test.go

@@ -199,6 +199,7 @@ func TestRenderCodexCommandExecutionItem(t *testing.T) {
 	item := codexThreadItemView{
 		Type:             "commandExecution",
 		Command:          "go test ./...",
+		CWD:              "/workspace/project",
 		AggregatedOutput: &output,
 		ExitCode:         &exitCode,
 	}
@@ -208,6 +209,11 @@ func TestRenderCodexCommandExecutionItem(t *testing.T) {
 			t.Fatalf("rendered command item missing %q in %q", want, text)
 		}
 	}
+	commandAt := strings.Index(text, "<b>Command</b>")
+	cwdAt := strings.Index(text, "<b>CWD:</b>")
+	if cwdAt >= 0 && commandAt > cwdAt {
+		t.Fatalf("command label should render before CWD to avoid Telegram attaching it to the CWD line: %q", text)
+	}
 }
 
 func TestRenderCodexStartedItems(t *testing.T) {
@@ -244,11 +250,40 @@ func TestRenderApprovalDetailsAvoidsRawJSONDump(t *testing.T) {
 			t.Fatalf("approval render missing %q in %q", want, text)
 		}
 	}
+	summary, _, _ := strings.Cut(text, "<blockquote expandable>")
+	for _, unwanted := range []string{"go test ./...", "/workspace/project"} {
+		if strings.Contains(summary, unwanted) {
+			t.Fatalf("approval summary should not include %q in %q", unwanted, summary)
+		}
+	}
 	if strings.Contains(text, "unused") {
 		t.Fatalf("approval render should omit unused JSON: %q", text)
 	}
 }
 
+func TestApprovalOnlyToolMessageCanReceiveCompletionDetails(t *testing.T) {
+	exitCode := 0
+	duration := int64(1234)
+	output := "done"
+	tool := toolMessageState{
+		approvalHTML: SummaryDetailsHTML("Codex requests command approval", "approval details"),
+	}
+	tool.toolHTML = renderCodexItemCompleted(codexThreadItemView{
+		Type:             "commandExecution",
+		Command:          "go test ./...",
+		CWD:              "/workspace/project",
+		ExitCode:         &exitCode,
+		DurationMs:       &duration,
+		AggregatedOutput: &output,
+	})
+	text := tool.html()
+	for _, want := range []string{"Tool call: command finished", "Exit code: 0", "Duration ms", "1234", "Codex requests command approval"} {
+		if !strings.Contains(text, want) {
+			t.Fatalf("combined approval tool message missing %q in %q", want, text)
+		}
+	}
+}
+
 func TestToolMessageAddsEditedAtBeforeDetails(t *testing.T) {
 	tool := toolMessageState{
 		toolHTML: SummaryDetailsHTML("Tool call: command finished\nCommand: go test ./...", "full output"),