From aa93b3f3a67436edd094dfff65b8bb61725c7e8f Mon Sep 17 00:00:00 2001 From: shanglei Date: Thu, 30 Jul 2026 14:26:00 +0800 Subject: [PATCH] fix(convertlib): keep an empty merge_forward body empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConvertBodyContent opened with `if ctx.RawContent == "" { return "" }` before the converters moved down to internal/imcontent. The guard went with them, and merge_forward is dispatched above that call — the shortcut-side converter expands the tree from the API rather than from body.content, so it never reaches imcontent's copy. A merge_forward item whose body.content is an empty string therefore stopped converting to "" the way every other message type does. With a prefetched page it renders a full subtree; without one, and with a runtime in hand, it issues an inline GET /open-apis/im/v1/messages/{id} and prints "[Merged forward: fetch failed: ...]" when that fails. Both reach the formatted message output. FormatEventMessage carries no Runtime and no prefetch, so the event path kept falling through to imcontent and was unaffected. The guard belongs above the dispatch, where it was, and now also covers a nil context, which the original would have dereferenced. Two tests pin it: the prefetch path must still convert to "", and the runtime path must issue zero requests. --- shortcuts/im/convert_lib/content_convert.go | 10 ++++ shortcuts/im/convert_lib/merge_test.go | 62 +++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/shortcuts/im/convert_lib/content_convert.go b/shortcuts/im/convert_lib/content_convert.go index 37c743141..0ba5aeea1 100644 --- a/shortcuts/im/convert_lib/content_convert.go +++ b/shortcuts/im/convert_lib/content_convert.go @@ -41,7 +41,17 @@ type ConvertContext struct { } // ConvertBodyContent converts body.content (a raw JSON string) to human-readable text. +// +// The empty-content guard has to live here, above the merge_forward dispatch, +// not only in imcontent.ConvertBodyContent: the shortcut-side merge_forward +// converter never consults imcontent on its expansion paths, so a guard that +// only sat below the dispatch would let an empty-content merge_forward render a +// prefetched tree — or issue an inline +// GET /open-apis/im/v1/messages/{id} — where every other message type returns "". func ConvertBodyContent(msgType string, ctx *ConvertContext) string { + if ctx == nil || ctx.RawContent == "" { + return "" + } if msgType == "merge_forward" { return (mergeForwardConverter{}).Convert(ctx) } diff --git a/shortcuts/im/convert_lib/merge_test.go b/shortcuts/im/convert_lib/merge_test.go index 2702fe0e4..9f7100d09 100644 --- a/shortcuts/im/convert_lib/merge_test.go +++ b/shortcuts/im/convert_lib/merge_test.go @@ -357,3 +357,65 @@ func TestFormatMergeForwardSubTreeInteractiveCardUsesMentions(t *testing.T) { t.Fatalf("FormatMergeForwardSubTree(interactive card) = %s", got) } } + +// TestConvertBodyContentEmptyMergeForwardStaysEmpty pins the empty-content +// guard that sits above the merge_forward dispatch. A merge_forward item whose +// body.content is an empty string (recalled/edited containers come back this +// way) must convert to "" — the same as every other message type — and must +// not reach the converter at all: reaching it would render a +// tree from a prefetched cache, or issue a +// GET /open-apis/im/v1/messages/{id} when no prefetch is present. +func TestConvertBodyContentEmptyMergeForwardStaysEmpty(t *testing.T) { + prefetch := map[string][]map[string]interface{}{ + "om_root": { + { + "message_id": "om_child", + "msg_type": "text", + "create_time": "1710500000000", + "sender": map[string]interface{}{"id": "ou_alice", "name": "Alice"}, + "body": map[string]interface{}{"content": `{"text":"hello"}`}, + }, + }, + } + + if got := ConvertBodyContent("merge_forward", &ConvertContext{ + MessageID: "om_root", + MergeForwardSubItems: prefetch, + }); got != "" { + t.Fatalf("ConvertBodyContent(merge_forward, empty content, prefetch hit) = %q, want empty", got) + } + + item := map[string]interface{}{ + "message_id": "om_root", + "msg_type": "merge_forward", + "create_time": "1710500000000", + "body": map[string]interface{}{"content": ""}, + } + msg := FormatMessageItemWithMergePrefetch(item, nil, nil, prefetch) + if got, _ := msg["content"].(string); got != "" { + t.Fatalf("FormatMessageItem(merge_forward, empty content, prefetch hit) content = %q, want empty", got) + } +} + +// TestConvertBodyContentEmptyMergeForwardIssuesNoRequest is the network half of +// the guard: with no prefetch and a live runtime, an empty-content +// merge_forward must not fall into the inline-fetch slow path. +func TestConvertBodyContentEmptyMergeForwardIssuesNoRequest(t *testing.T) { + var requests int + runtime := newBotConvertlibRuntime(t, convertlibRoundTripFunc(func(req *http.Request) (*http.Response, error) { + requests++ + return nil, fmt.Errorf("unexpected request: %s", req.URL.String()) + })) + + got := ConvertBodyContent("merge_forward", &ConvertContext{ + MessageID: "om_root", + Runtime: runtime, + SenderNames: map[string]string{}, + }) + if got != "" { + t.Fatalf("ConvertBodyContent(merge_forward, empty content, runtime set) = %q, want empty", got) + } + if requests != 0 { + t.Fatalf("ConvertBodyContent(merge_forward, empty content) issued %d HTTP request(s), want 0", requests) + } +}