fix: improve ack reaction logic and Feishu message handling

- Fix group-mentions scope to properly check IsGroup
- Add allowlist check in Feishu message handler
- Fix mention detection to only apply in direct chats
- Remove remove_ack_after_reply configuration option
- Change default ack_reaction to empty string (disabled by default)
- Add comprehensive unit tests for ack_reactions

config/config.example.json

@@ -130,9 +130,8 @@
     "monitor_usb": true
   },
   "messages": {
-    "ack_reaction": "OK",
-    "ack_reaction_scope": "group-mentions",
-    "remove_ack_after_reply": false
+    "ack_reaction": "",
+    "ack_reaction_scope": "group-mentions"
   },
   "gateway": {
     "host": "0.0.0.0",

pkg/channels/ack_reactions.go

@@ -69,6 +69,10 @@ func ShouldAckReaction(params AckReactionParams) bool {
 
 	// Group mentions only
 	if scope == AckReactionScopeGroupMentions {
+		// Not a group message, don't ack
+		if !params.IsGroup {
+			return false
+		}
 		// Not a mentionable group, don't ack
 		if !params.IsMentionableGroup {
 			return false
@@ -90,8 +94,6 @@ func ShouldAckReaction(params AckReactionParams) bool {
 
 // AckReactionManager manages the lifecycle of acknowledgment reactions
 type AckReactionManager struct {
-	// RemoveAfterReply indicates whether to remove the ack after reply
-	RemoveAfterReply bool
 	// ReactionValue is the current reaction value (emoji)
 	ReactionValue string
 	// Added indicates if the ack reaction has been added
@@ -99,11 +101,10 @@ type AckReactionManager struct {
 }
 
 // NewAckReactionManager creates a new ack reaction manager
-func NewAckReactionManager(removeAfterReply bool, reaction string) *AckReactionManager {
+func NewAckReactionManager(reaction string) *AckReactionManager {
 	return &AckReactionManager{
-		RemoveAfterReply: removeAfterReply,
-		ReactionValue:    reaction,
-		Added:            false,
+		ReactionValue: reaction,
+		Added:         false,
 	}
 }
 
@@ -111,8 +112,3 @@ func NewAckReactionManager(removeAfterReply bool, reaction string) *AckReactionM
 func (m *AckReactionManager) MarkAdded() {
 	m.Added = true
 }
-
-// ShouldRemoveAfterReply determines whether to remove the ack after reply
-func (m *AckReactionManager) ShouldRemoveAfterReply() bool {
-	return m.RemoveAfterReply && m.Added && m.ReactionValue != ""
-}

pkg/channels/ack_reactions_test.go

@@ -0,0 +1,196 @@
+package channels
+
+import "testing"
+
+func TestShouldAckReaction(t *testing.T) {
+	tests := []struct {
+		name   string
+		params AckReactionParams
+		want   bool
+	}{
+		// Scope: off/none (disabled)
+		{
+			name:   "scope off disables all",
+			params: AckReactionParams{Scope: AckReactionScopeOff, IsDirect: true, IsGroup: true},
+			want:   false,
+		},
+		{
+			name:   "scope none alias disables all",
+			params: AckReactionParams{Scope: AckReactionScopeNone, IsDirect: true, IsGroup: true},
+			want:   false,
+		},
+		// Scope: all
+		{
+			name:   "scope all enables for direct messages",
+			params: AckReactionParams{Scope: AckReactionScopeAll, IsDirect: true, IsGroup: false},
+			want:   true,
+		},
+		{
+			name:   "scope all enables for group messages",
+			params: AckReactionParams{Scope: AckReactionScopeAll, IsDirect: false, IsGroup: true},
+			want:   true,
+		},
+		// Scope: direct
+		{
+			name:   "scope direct enables for direct messages",
+			params: AckReactionParams{Scope: AckReactionScopeDirect, IsDirect: true, IsGroup: false},
+			want:   true,
+		},
+		{
+			name:   "scope direct disables for group messages",
+			params: AckReactionParams{Scope: AckReactionScopeDirect, IsDirect: false, IsGroup: true},
+			want:   false,
+		},
+		// Scope: group-all
+		{
+			name:   "scope group-all enables for group messages",
+			params: AckReactionParams{Scope: AckReactionScopeGroupAll, IsDirect: false, IsGroup: true},
+			want:   true,
+		},
+		{
+			name:   "scope group-all disables for direct messages",
+			params: AckReactionParams{Scope: AckReactionScopeGroupAll, IsDirect: true, IsGroup: false},
+			want:   false,
+		},
+		// Scope: group-mentions (mentionable group, require mention, can detect)
+		{
+			name: "scope group-mentions with all conditions met",
+			params: AckReactionParams{
+				Scope:              AckReactionScopeGroupMentions,
+				IsDirect:           false,
+				IsGroup:            true,
+				IsMentionableGroup: true,
+				RequireMention:     true,
+				CanDetectMention:   true,
+				WasMentioned:       true,
+			},
+			want: true,
+		},
+		{
+			name: "scope group-mentions bypass mention requirement",
+			params: AckReactionParams{
+				Scope:               AckReactionScopeGroupMentions,
+				IsDirect:            false,
+				IsGroup:             true,
+				IsMentionableGroup:  true,
+				RequireMention:      true,
+				CanDetectMention:    true,
+				ShouldBypassMention: true,
+			},
+			want: true,
+		},
+		{
+			name: "scope group-mentions not in group",
+			params: AckReactionParams{
+				Scope:              AckReactionScopeGroupMentions,
+				IsDirect:           true,
+				IsGroup:            false,
+				IsMentionableGroup: true,
+				RequireMention:     true,
+				CanDetectMention:   true,
+				WasMentioned:       true,
+			},
+			want: false,
+		},
+		{
+			name: "scope group-mentions not mentionable group",
+			params: AckReactionParams{
+				Scope:              AckReactionScopeGroupMentions,
+				IsDirect:           false,
+				IsGroup:            true,
+				IsMentionableGroup: false,
+				RequireMention:     true,
+				CanDetectMention:   true,
+				WasMentioned:       true,
+			},
+			want: false,
+		},
+		{
+			name: "scope group-mentions no mention required",
+			params: AckReactionParams{
+				Scope:              AckReactionScopeGroupMentions,
+				IsDirect:           false,
+				IsGroup:            true,
+				IsMentionableGroup: true,
+				RequireMention:     false,
+				CanDetectMention:   true,
+				WasMentioned:       true,
+			},
+			want: false,
+		},
+		{
+			name: "scope group-mentions cannot detect mention",
+			params: AckReactionParams{
+				Scope:              AckReactionScopeGroupMentions,
+				IsDirect:           false,
+				IsGroup:            true,
+				IsMentionableGroup: true,
+				RequireMention:     true,
+				CanDetectMention:   false,
+				WasMentioned:       true,
+			},
+			want: false,
+		},
+		{
+			name: "scope group-mentions not mentioned",
+			params: AckReactionParams{
+				Scope:              AckReactionScopeGroupMentions,
+				IsDirect:           false,
+				IsGroup:            true,
+				IsMentionableGroup: true,
+				RequireMention:     true,
+				CanDetectMention:   true,
+				WasMentioned:       false,
+			},
+			want: false,
+		},
+		// Default scope (empty) - should default to group-mentions
+		{
+			name: "empty scope defaults to group-mentions with all conditions met",
+			params: AckReactionParams{
+				Scope:              "",
+				IsDirect:           false,
+				IsGroup:            true,
+				IsMentionableGroup: true,
+				RequireMention:     true,
+				CanDetectMention:   true,
+				WasMentioned:       true,
+			},
+			want: true,
+		},
+		{
+			name: "empty scope defaults to group-mentions not in group",
+			params: AckReactionParams{
+				Scope:              "",
+				IsDirect:           true,
+				IsGroup:            false,
+				IsMentionableGroup: true,
+				RequireMention:     true,
+				CanDetectMention:   true,
+				WasMentioned:       true,
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ShouldAckReaction(tt.params); got != tt.want {
+				t.Errorf("ShouldAckReaction() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestAckReactionManager(t *testing.T) {
+	t.Run("mark added", func(t *testing.T) {
+		m := NewAckReactionManager("OK")
+		if m.Added {
+			t.Error("expected Added to be false initially")
+		}
+		m.MarkAdded()
+		if !m.Added {
+			t.Error("expected Added to be true after MarkAdded")
+		}
+	})
+}

pkg/channels/feishu_64.go

@@ -151,14 +151,24 @@ func (c *FeishuChannel) handleMessageReceive(ctx context.Context, event *larkim.
 		content = "[empty message]"
 	}
 
+	// Check allowlist before processing
+	if !c.IsAllowed(senderID) {
+		logger.DebugCF("feishu", "Message blocked by allowlist", map[string]interface{}{
+			"sender_id": senderID,
+		})
+		return nil
+	}
+
 	// Determine chat type: p2p = direct, group = group chat
 	chatType := stringValue(message.ChatType)
 	isGroup := chatType == "group"
 	isDirect := chatType == "p2p"
 
 	// Check if bot was mentioned
+	// Without a reliable bot ID in scope, we conservatively only
+	// treat mentions in direct (p2p) chats as relevant to the bot.
 	wasMentioned := false
-	if message.Mentions != nil && len(message.Mentions) > 0 {
+	if isDirect && message.Mentions != nil && len(message.Mentions) > 0 {
 		wasMentioned = true
 	}
 

pkg/config/config.go

@@ -171,10 +171,8 @@ type DevicesConfig struct {
 type MessagesConfig struct {
 	// AckReaction is the emoji used to acknowledge inbound messages (empty to disable)
 	AckReaction string `json:"ack_reaction" env:"PICOCLAW_MESSAGES_ACK_REACTION"`
-	// AckReactionScope controls when to send ack reactions: "all", "direct", "group-all", "group-mentions", "off"
+	// AckReactionScope controls when to send ack reactions: "all", "direct", "group-all", "group-mentions", "off" (or "none" to disable)
 	AckReactionScope string `json:"ack_reaction_scope" env:"PICOCLAW_MESSAGES_ACK_REACTION_SCOPE"`
-	// RemoveAckAfterReply removes the ack reaction after reply is sent
-	RemoveAckAfterReply bool `json:"remove_ack_after_reply" env:"PICOCLAW_MESSAGES_REMOVE_ACK_AFTER_REPLY"`
 }
 
 type ProvidersConfig struct {
@@ -343,9 +341,8 @@ func DefaultConfig() *Config {
 			MonitorUSB: true,
 		},
 		Messages: MessagesConfig{
-			AckReaction:         "OK",
-			AckReactionScope:    "group-mentions",
-			RemoveAckAfterReply: false,
+			AckReaction:      "",
+			AckReactionScope: "group-mentions",
 		},
 	}
 }