feat(slack): admit bot @mentions via allow_bot_ids, drop self always (closes #55)

forge-cli/templates/init/slack-config.yaml.tmpl

@@ -2,3 +2,12 @@ adapter: slack
 settings:
   app_token_env: SLACK_APP_TOKEN
   bot_token_env: SLACK_BOT_TOKEN
+
+  # Optional: comma-separated list of bot_ids whose @mentions of this agent
+  # should be honored. By default no other bots are admitted, so the agent
+  # only responds to humans. Use this to let a scheduler, monitoring tool,
+  # or CI bot mention the agent. The agent's own bot_id is always dropped
+  # regardless of this list (self-loop guard).
+  #
+  # Find a bot's bot_id: Slack admin → Manage apps → app → Bot User OAuth.
+  # allow_bot_ids: B0123ABC,B0456DEF

forge-plugins/channels/slack/slack.go

@@ -29,16 +29,18 @@ const longRunningThreshold = 15 * time.Second
 
 // Plugin implements channels.ChannelPlugin for Slack using Socket Mode.
 type Plugin struct {
-	appToken   string
-	botToken   string
-	botUserID  string // resolved at startup via auth.test
-	wsConn     *websocket.Conn
-	connMu     sync.Mutex
-	stopCh     chan struct{}
-	client     *http.Client
-	apiBase    string // overridable for tests
-	dedupMu    sync.Mutex
-	dedupCache map[string]time.Time
+	appToken    string
+	botToken    string
+	botUserID   string          // resolved at startup via auth.test
+	ownBotID    string          // resolved at startup via auth.test; used as the self-loop guard
+	allowBotIDs map[string]bool // bot_ids whose @mentions are admitted; default empty (no other bots admitted)
+	wsConn      *websocket.Conn
+	connMu      sync.Mutex
+	stopCh      chan struct{}
+	client      *http.Client
+	apiBase     string // overridable for tests
+	dedupMu     sync.Mutex
+	dedupCache  map[string]time.Time
 }
 
 // New creates an uninitialised Slack plugin.
@@ -64,10 +66,52 @@ func (p *Plugin) Init(cfg channels.ChannelConfig) error {
 		return fmt.Errorf("slack: bot_token is required (set SLACK_BOT_TOKEN)")
 	}
 
+	// Optional: comma-separated list of bot_ids whose @mentions of the agent
+	// are admitted. Empty (the default) means no other bots are admitted —
+	// the agent only responds to humans. The agent's own bot_id is always
+	// dropped regardless of this list (see ownBotID guard in Start).
+	p.allowBotIDs = parseAllowBotIDs(settings["allow_bot_ids"])
+
 	return nil
 }
 
-// resolveBotID calls auth.test to discover the bot's own Slack user ID.
+// parseAllowBotIDs splits a comma-separated bot_id list into a lookup set.
+// Whitespace around entries is trimmed; empty entries are skipped.
+func parseAllowBotIDs(raw string) map[string]bool {
+	set := make(map[string]bool)
+	for _, id := range strings.Split(raw, ",") {
+		id = strings.TrimSpace(id)
+		if id != "" {
+			set[id] = true
+		}
+	}
+	return set
+}
+
+// admitBotEvent decides whether an inbound event authored by a bot should
+// flow through to the agent. Human messages (botID == "") always admit.
+// The agent's own bot_id is dropped unconditionally — this is the self-loop
+// guard, not subject to the allowlist. Any other bot is admitted only when
+// its bot_id appears in allowBotIDs.
+//
+// The returned reason string is the operator-facing log line for dropped
+// events; for admitted events it is empty.
+func (p *Plugin) admitBotEvent(botID string) (reason string, admit bool) {
+	if botID == "" {
+		return "", true
+	}
+	if botID == p.ownBotID {
+		return fmt.Sprintf("dropping event authored by self (bot_id=%s)", botID), false
+	}
+	if !p.allowBotIDs[botID] {
+		return fmt.Sprintf("dropping event from non-allowlisted bot (bot_id=%s); add to slack-config.yaml allow_bot_ids to admit", botID), false
+	}
+	return "", true
+}
+
+// resolveBotID calls auth.test to discover the bot's own Slack user ID and
+// bot_id. The user ID drives @mention matching; the bot_id powers the
+// self-loop guard that drops messages authored by this same bot.
 func (p *Plugin) resolveBotID() error {
 	req, err := http.NewRequest(http.MethodPost, p.apiBase+"/auth.test", nil)
 	if err != nil {
@@ -90,6 +134,7 @@ func (p *Plugin) resolveBotID() error {
 	var result struct {
 		OK     bool   `json:"ok"`
 		UserID string `json:"user_id"`
+		BotID  string `json:"bot_id"`
 		Error  string `json:"error,omitempty"`
 	}
 	if err := json.Unmarshal(body, &result); err != nil {
@@ -100,6 +145,7 @@ func (p *Plugin) resolveBotID() error {
 	}
 
 	p.botUserID = result.UserID
+	p.ownBotID = result.BotID
 	return nil
 }
 
@@ -325,8 +371,11 @@ func (p *Plugin) readLoop(ctx context.Context, conn *websocket.Conn, handler cha
 			continue
 		}
 
-		// Skip bot messages.
-		if payload.Event.BotID != "" {
+		// Bot-authored events go through admitBotEvent: self-mentions are
+		// always dropped (loop guard); other bots are dropped unless the
+		// operator has admitted them via allow_bot_ids in slack-config.yaml.
+		if reason, admit := p.admitBotEvent(payload.Event.BotID); !admit {
+			fmt.Printf("  slack: %s\n", reason)
 			continue
 		}
 

forge-plugins/channels/slack/slack_test.go

@@ -467,7 +467,9 @@ func TestResolveBotID(t *testing.T) {
 			t.Errorf("Authorization = %q, want 'Bearer xoxb-test-token'", r.Header.Get("Authorization"))
 		}
 		w.Header().Set("Content-Type", "application/json")
-		w.Write([]byte(`{"ok":true,"user_id":"U123BOT"}`)) //nolint:errcheck
+		// auth.test returns both user_id and bot_id; the plugin needs both —
+		// user_id for @mention matching, bot_id for the self-loop guard.
+		w.Write([]byte(`{"ok":true,"user_id":"U123BOT","bot_id":"B123BOT"}`)) //nolint:errcheck
 	}))
 	defer srv.Close()
 
@@ -482,6 +484,9 @@ func TestResolveBotID(t *testing.T) {
 	if p.botUserID != "U123BOT" {
 		t.Errorf("botUserID = %q, want U123BOT", p.botUserID)
 	}
+	if p.ownBotID != "B123BOT" {
+		t.Errorf("ownBotID = %q, want B123BOT", p.ownBotID)
+	}
 }
 
 func TestResolveBotID_Error(t *testing.T) {
@@ -788,3 +793,118 @@ func TestEvictExpiredDedup(t *testing.T) {
 		t.Error("recent-env should still be present")
 	}
 }
+
+// --- Bot-mention admission (issue #55) -----------------------------------
+
+func TestParseAllowBotIDs(t *testing.T) {
+	tests := []struct {
+		name string
+		raw  string
+		want []string // sorted list of ids expected in the set
+	}{
+		{"empty", "", nil},
+		{"single", "B0123ABC", []string{"B0123ABC"}},
+		{"two with spaces", " B0123ABC , B0456DEF ", []string{"B0123ABC", "B0456DEF"}},
+		{"empty entries ignored", "B0123ABC,,B0456DEF,", []string{"B0123ABC", "B0456DEF"}},
+		{"whitespace only ignored", "  ,   ,  ", nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := parseAllowBotIDs(tt.raw)
+			if len(got) != len(tt.want) {
+				t.Fatalf("got %d entries, want %d (entries=%v)", len(got), len(tt.want), got)
+			}
+			for _, id := range tt.want {
+				if !got[id] {
+					t.Errorf("missing %q in result", id)
+				}
+			}
+		})
+	}
+}
+
+func TestInit_PopulatesAllowBotIDs(t *testing.T) {
+	p := New()
+	err := p.Init(channels.ChannelConfig{
+		Adapter: "slack",
+		Settings: map[string]string{
+			"app_token":     "xoxa-test",
+			"bot_token":     "xoxb-test",
+			"allow_bot_ids": "B0123ABC, B0456DEF",
+		},
+	})
+	if err != nil {
+		t.Fatalf("Init: %v", err)
+	}
+	if !p.allowBotIDs["B0123ABC"] || !p.allowBotIDs["B0456DEF"] {
+		t.Errorf("allowBotIDs = %v, want both B0123ABC and B0456DEF admitted", p.allowBotIDs)
+	}
+}
+
+func TestInit_AllowBotIDsAbsent(t *testing.T) {
+	// Default behavior: with no allow_bot_ids setting, the allowlist is
+	// empty and only humans (botID == "") flow through admitBotEvent.
+	p := New()
+	err := p.Init(channels.ChannelConfig{
+		Adapter: "slack",
+		Settings: map[string]string{
+			"app_token": "xoxa-test",
+			"bot_token": "xoxb-test",
+		},
+	})
+	if err != nil {
+		t.Fatalf("Init: %v", err)
+	}
+	if len(p.allowBotIDs) != 0 {
+		t.Errorf("expected empty allowBotIDs, got %v", p.allowBotIDs)
+	}
+}
+
+func TestAdmitBotEvent(t *testing.T) {
+	p := New()
+	p.ownBotID = "B0SELF"
+	p.allowBotIDs = map[string]bool{"B0ALLOWED": true}
+
+	tests := []struct {
+		name      string
+		botID     string
+		wantAdmit bool
+		// wantReasonSubstr is checked when admit is false to make sure the
+		// log line is operator-actionable.
+		wantReasonSubstr string
+	}{
+		{"human message admitted", "", true, ""},
+		{"own bot_id dropped (self-loop guard)", "B0SELF", false, "authored by self"},
+		{"allowlisted bot admitted", "B0ALLOWED", true, ""},
+		{"non-allowlisted bot dropped", "B0OTHER", false, "non-allowlisted"},
+		{"non-allowlisted bot reason mentions allow_bot_ids", "B0OTHER", false, "allow_bot_ids"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			reason, admit := p.admitBotEvent(tt.botID)
+			if admit != tt.wantAdmit {
+				t.Errorf("admit = %v, want %v (reason=%q)", admit, tt.wantAdmit, reason)
+			}
+			if !admit && !strings.Contains(reason, tt.wantReasonSubstr) {
+				t.Errorf("reason = %q, want substring %q", reason, tt.wantReasonSubstr)
+			}
+		})
+	}
+}
+
+// TestAdmitBotEvent_SelfGuardBeatsAllowlist verifies the hard rule from #55:
+// even if the agent's own bot_id were somehow listed in allow_bot_ids,
+// the self-loop guard short-circuits before the allowlist check.
+func TestAdmitBotEvent_SelfGuardBeatsAllowlist(t *testing.T) {
+	p := New()
+	p.ownBotID = "B0SELF"
+	p.allowBotIDs = map[string]bool{"B0SELF": true} // misconfiguration
+
+	reason, admit := p.admitBotEvent("B0SELF")
+	if admit {
+		t.Fatal("agent must never admit its own bot_id, even when allowlisted")
+	}
+	if !strings.Contains(reason, "self") {
+		t.Errorf("reason should identify self-loop guard, got %q", reason)
+	}
+}