From 7c7d7146ef8742372da7505d885a528fff697a4d Mon Sep 17 00:00:00 2001 From: maxiaoyang <2768753269@qq.com> Date: Wed, 29 Apr 2026 21:01:03 +0800 Subject: [PATCH] feat(messageutil): add IsSystemSenderID and ApplyUserNamePrefix helpers Two more helpers needed for end-to-end multi-user attribution: - IsSystemSenderID filters synthetic trigger sources (cron, heartbeat, async:* tool callbacks, system channel) so internal events do not surface as distinct human users in the conversation. Without this, cron-triggered messages would carry attribution like cron, producing a confusing [cron] [System: cron] ... double prefix on adapters that fall back to in-content rendering. - ApplyUserNamePrefix renders msg.Name as a [name] prefix on user content for adapters whose APIs do not support a per-message author identity (Anthropic, Bedrock, OpenAI Responses). The persisted message is never mutated; only the wire payload carries the prefix. Tool result messages and non-user roles pass through unchanged. These helpers are pure and independently testable; they will be wired into the agent and provider adapters in subsequent commits. Refs #2702. --- pkg/providers/messageutil/messageutil.go | 30 ++++++ pkg/providers/messageutil/messageutil_test.go | 91 +++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/pkg/providers/messageutil/messageutil.go b/pkg/providers/messageutil/messageutil.go index 9ad8ec71f..cfa57cc5d 100644 --- a/pkg/providers/messageutil/messageutil.go +++ b/pkg/providers/messageutil/messageutil.go @@ -1,6 +1,7 @@ package messageutil import ( + "fmt" "regexp" "strings" @@ -38,6 +39,35 @@ func SanitizeMessageName(raw string) string { return cleaned } +// IsSystemSenderID reports whether senderID identifies an internal trigger +// (cron, heartbeat, async tool callback, system channel) rather than a real +// human user. These should not propagate as message-level sender attribution +// because they don't represent distinct actors in a multi-user conversation. +func IsSystemSenderID(senderID string) bool { + id := strings.ToLower(strings.TrimSpace(senderID)) + switch id { + case "", "cron", "heartbeat", "system": + return true + } + return strings.HasPrefix(id, "async:") +} + +// ApplyUserNamePrefix returns the message content with a `[name] ` prefix +// when msg carries sender attribution that the calling adapter cannot send +// natively (Anthropic, Bedrock, etc.). The persisted msg is not mutated. +// +// Returns msg.Content unchanged when: +// - the role is not "user" +// - msg.Name is empty +// - msg.ToolCallID is set (tool result, not a user utterance) +// - msg.Content is empty (avoid producing a bare "[name] ") +func ApplyUserNamePrefix(msg protocoltypes.Message) string { + if msg.Role != "user" || msg.Name == "" || msg.ToolCallID != "" || msg.Content == "" { + return msg.Content + } + return fmt.Sprintf("[%s] %s", msg.Name, msg.Content) +} + // IsTransientAssistantThoughtMessage reports whether msg is an invalid // reasoning-only assistant history record. These "hanging" thought messages // are not a canonical persisted format and should be discarded instead of diff --git a/pkg/providers/messageutil/messageutil_test.go b/pkg/providers/messageutil/messageutil_test.go index c890da3bb..30512322d 100644 --- a/pkg/providers/messageutil/messageutil_test.go +++ b/pkg/providers/messageutil/messageutil_test.go @@ -6,8 +6,11 @@ package messageutil import ( + "reflect" "strings" "testing" + + "github.com/sipeed/picoclaw/pkg/providers/protocoltypes" ) func TestSanitizeMessageName(t *testing.T) { @@ -65,3 +68,91 @@ func TestSanitizeMessageName_OutputAlwaysWireSafe(t *testing.T) { } } } + +func TestIsSystemSenderID(t *testing.T) { + tests := []struct { + in string + want bool + }{ + {"", true}, + {" ", true}, + {"cron", true}, + {"CRON", true}, + {"heartbeat", true}, + {"system", true}, + {"async:tool_call", true}, + {"ASYNC:Foo", true}, + {"alice", false}, + {"U07AB12C3DEF", false}, + {"141455495", false}, + {"alice#1234", false}, + } + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + if got := IsSystemSenderID(tt.in); got != tt.want { + t.Errorf("IsSystemSenderID(%q) = %v, want %v", tt.in, got, tt.want) + } + }) + } +} + +func TestApplyUserNamePrefix(t *testing.T) { + tests := []struct { + name string + msg protocoltypes.Message + want string + }{ + { + name: "user with name", + msg: protocoltypes.Message{Role: "user", Name: "alice", Content: "hello"}, + want: "[alice] hello", + }, + { + name: "user without name", + msg: protocoltypes.Message{Role: "user", Content: "hello"}, + want: "hello", + }, + { + name: "user with name but empty content", + msg: protocoltypes.Message{Role: "user", Name: "alice", Content: ""}, + want: "", + }, + { + name: "user tool result is not prefixed", + msg: protocoltypes.Message{Role: "user", Name: "alice", Content: `{"ok":true}`, ToolCallID: "call_1"}, + want: `{"ok":true}`, + }, + { + name: "assistant with name not prefixed", + msg: protocoltypes.Message{Role: "assistant", Name: "alice", Content: "I am the assistant"}, + want: "I am the assistant", + }, + { + name: "tool role with name not prefixed", + msg: protocoltypes.Message{Role: "tool", Name: "alice", Content: `{"x":1}`}, + want: `{"x":1}`, + }, + { + name: "system role with name not prefixed", + msg: protocoltypes.Message{Role: "system", Name: "alice", Content: "instructions"}, + want: "instructions", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ApplyUserNamePrefix(tt.msg) + if got != tt.want { + t.Errorf("ApplyUserNamePrefix(%+v) = %q, want %q", tt.msg, got, tt.want) + } + }) + } +} + +func TestApplyUserNamePrefix_DoesNotMutateInput(t *testing.T) { + msg := protocoltypes.Message{Role: "user", Name: "alice", Content: "hello"} + original := msg + _ = ApplyUserNamePrefix(msg) + if !reflect.DeepEqual(msg, original) { + t.Errorf("ApplyUserNamePrefix mutated the input message: got %+v, want %+v", msg, original) + } +}