From 5040fe4cbec5c56b54eb42e681b0b0b498b922a8 Mon Sep 17 00:00:00 2001 From: viettranx Date: Wed, 15 Apr 2026 20:37:33 +0700 Subject: [PATCH] fix(channels): suppress agent-error text on public-facing channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework of #911 against dev with correctness fixes. Public-facing channels (Facebook, Telegram, Discord, Feishu, WhatsApp, Zalo OA, Zalo Personal, Pancake, Slack) no longer receive raw internal error text when an agent run fails — an empty outbound is published instead so channels still clean up placeholders / typing indicators. Errors continue to be logged server-side at Error level. Applied on both hot paths: - cmd/gateway_consumer_normal.go (agent run failure) - cmd/gateway_subagent_announce_queue.go (announce lead run failure) Fixes vs #911: - Whitelist uses channels.Type* constants (compile-time safe) instead of string literals. The original PR's literals ("zalo", "line") did not match any real channel type, so Zalo OA/Personal and Pancake leaked errors while "line" was a dead branch. - Added TypePancake and TypeSlack which were missing. - Empty Content in Facebook.Send and Pancake.Send now short-circuits (matching Telegram/Discord/Slack). Previously, suppressed errors flowed to Graph/Pancake APIs as empty payloads, triggering 400s and channel health degradation. - Added table-driven test for isExternalChannel covering all real channel types + empty + unknown + legacy "zalo" short form. Closes #911 --- cmd/gateway_consumer_normal.go | 11 +++++- cmd/gateway_errors.go | 21 +++++++++++ cmd/gateway_errors_test.go | 49 ++++++++++++++++++++++++++ cmd/gateway_subagent_announce_queue.go | 8 ++++- internal/channels/facebook/facebook.go | 8 +++++ internal/channels/pancake/pancake.go | 7 ++++ 6 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 cmd/gateway_errors_test.go diff --git a/cmd/gateway_consumer_normal.go b/cmd/gateway_consumer_normal.go index 85313c5f..a3167b80 100644 --- a/cmd/gateway_consumer_normal.go +++ b/cmd/gateway_consumer_normal.go @@ -415,10 +415,19 @@ func processNormalMessage( return } slog.Error("inbound: agent run failed", "error", outcome.Err, "channel", channel) + // Suppress technical error text on public-facing channels (FB, Telegram, etc.) + // Empty Content still triggers placeholder/typing cleanup downstream. + errContent := formatAgentError(outcome.Err) + if deps.ChannelMgr != nil { + if ct := deps.ChannelMgr.ChannelTypeForName(channel); isExternalChannel(ct) { + slog.Info("inbound: suppressed error for external channel", "channel", channel, "type", ct) + errContent = "" + } + } deps.MsgBus.PublishOutbound(bus.OutboundMessage{ Channel: channel, ChatID: chatID, - Content: formatAgentError(outcome.Err), + Content: errContent, Metadata: meta, }) return diff --git a/cmd/gateway_errors.go b/cmd/gateway_errors.go index 95ee5db1..61dde32b 100644 --- a/cmd/gateway_errors.go +++ b/cmd/gateway_errors.go @@ -74,6 +74,27 @@ func isContextOverflowError(lower string) bool { containsAny(lower, "overflow", "too large", "too long", "limit", "exceeded")) } +// isExternalChannel reports whether a channel type serves end users on a +// public-facing platform (Facebook, Telegram, etc.). Internal error details +// must not be forwarded to these channels — the caller publishes an empty +// outbound instead so placeholders get cleaned up without leaking technical +// error text to end users. Internal types ("ws", "") return false. +func isExternalChannel(channelType string) bool { + switch channelType { + case channels.TypeFacebook, + channels.TypeTelegram, + channels.TypeDiscord, + channels.TypeFeishu, + channels.TypeWhatsApp, + channels.TypeZaloOA, + channels.TypeZaloPersonal, + channels.TypePancake, + channels.TypeSlack: + return true + } + return false +} + // isMessageFormatError checks for tool_use/tool_result mismatch, role ordering, // and other message format errors that indicate corrupted session history. func isMessageFormatError(lower string) bool { diff --git a/cmd/gateway_errors_test.go b/cmd/gateway_errors_test.go new file mode 100644 index 00000000..6efcf484 --- /dev/null +++ b/cmd/gateway_errors_test.go @@ -0,0 +1,49 @@ +package cmd + +import ( + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/channels" +) + +// TestIsExternalChannel ensures the whitelist matches actual channel type +// constants — a mismatch here silently leaks internal error strings to end +// users (e.g. "zalo" vs "zalo_oa"/"zalo_personal"). Always compare against +// channels.Type* constants, never string literals. +func TestIsExternalChannel(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + channelType string + want bool + }{ + // Public-facing platforms — errors must be suppressed. + {"facebook", channels.TypeFacebook, true}, + {"telegram", channels.TypeTelegram, true}, + {"discord", channels.TypeDiscord, true}, + {"feishu", channels.TypeFeishu, true}, + {"whatsapp", channels.TypeWhatsApp, true}, + {"zalo_oa", channels.TypeZaloOA, true}, + {"zalo_personal", channels.TypeZaloPersonal, true}, + {"pancake", channels.TypePancake, true}, + {"slack", channels.TypeSlack, true}, + + // Internal / unknown channel types — errors must still surface. + {"ws", "ws", false}, + {"empty", "", false}, + {"unknown", "myplatform", false}, + // "line" is a Pancake sub-platform, not a top-level channel type. + {"line_not_top_level", "line", false}, + // Legacy short "zalo" string must NOT match — real constants are zalo_oa / zalo_personal. + {"zalo_short_form", "zalo", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isExternalChannel(tc.channelType); got != tc.want { + t.Fatalf("isExternalChannel(%q) = %v, want %v", tc.channelType, got, tc.want) + } + }) + } +} diff --git a/cmd/gateway_subagent_announce_queue.go b/cmd/gateway_subagent_announce_queue.go index 87264d54..1aef9b95 100644 --- a/cmd/gateway_subagent_announce_queue.go +++ b/cmd/gateway_subagent_announce_queue.go @@ -182,10 +182,16 @@ func processSubagentAnnounceLoop( if outcome.Err != nil { if !errors.Is(outcome.Err, context.Canceled) { slog.Error("subagent announce: lead run failed", "error", outcome.Err, "batch_size", len(entries)) + errContent := formatAgentError(outcome.Err) + if isExternalChannel(r.OrigChannelType) { + slog.Info("subagent announce: suppressed error for external channel", + "channel", r.OrigChannel, "type", r.OrigChannelType) + errContent = "" + } msgBus.PublishOutbound(bus.OutboundMessage{ Channel: r.OrigChannel, ChatID: r.OrigChatID, - Content: formatAgentError(outcome.Err), + Content: errContent, Metadata: r.OutMeta, }) } diff --git a/internal/channels/facebook/facebook.go b/internal/channels/facebook/facebook.go index 5b0ba32a..a8d3b82f 100644 --- a/internal/channels/facebook/facebook.go +++ b/internal/channels/facebook/facebook.go @@ -198,6 +198,14 @@ func (ch *Channel) isBotEcho(chatID string, eventAt time.Time) bool { // Send delivers an outbound message. Dispatches to comment reply or Messenger based on fb_mode metadata. func (ch *Channel) Send(ctx context.Context, msg bus.OutboundMessage) error { + // NO_REPLY / suppressed-error path: empty content with no media means the + // caller wants downstream cleanup (placeholder, typing) but no user-visible + // message. Graph API rejects empty text, so short-circuit here — matches + // the pattern used by Telegram, Discord and Slack. + if msg.Content == "" && len(msg.Media) == 0 { + return nil + } + mode := msg.Metadata["fb_mode"] switch mode { diff --git a/internal/channels/pancake/pancake.go b/internal/channels/pancake/pancake.go index 0ab40bff..f7bdb4e1 100644 --- a/internal/channels/pancake/pancake.go +++ b/internal/channels/pancake/pancake.go @@ -184,6 +184,13 @@ func (ch *Channel) Send(ctx context.Context, msg bus.OutboundMessage) error { return fmt.Errorf("pancake: chat_id (conversation_id) is required for outbound message") } + // NO_REPLY / suppressed-error path: empty content with no media means the + // caller wants downstream cleanup only. Pancake API rejects empty payloads, + // so short-circuit before dispatch. + if msg.Content == "" && len(msg.Media) == 0 { + return nil + } + switch msg.Metadata["pancake_mode"] { case "comment": return ch.sendCommentReply(ctx, msg)