Skip to content

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

Open
initializ-mk wants to merge 1 commit into
mainfrom
enhancement/slack-bot-mentions
Open

feat(slack): admit bot @mentions via allow_bot_ids, drop self always (closes #55)#56
initializ-mk wants to merge 1 commit into
mainfrom
enhancement/slack-bot-mentions

Conversation

@initializ-mk
Copy link
Copy Markdown
Contributor

Summary

Fixes #55 — the Slack adapter previously dropped every event with bot_id != "", so a forge-agent that responded to human @-mentions stayed silent when a scheduler, monitoring tool, workflow step, or CI bot routed a mention to it. Lifting that filter naively would let the agent respond to its own messages and hot-loop.

This PR replaces the binary filter with two coordinated rules:

  1. Self-loop guard (unconditional). The agent's own bot_id is dropped before the allowlist is even consulted. No opt-out — even if ownBotID is misconfigured into allow_bot_ids, the guard short-circuits.
  2. Operator-controlled allowlist (opt-in). A new allow_bot_ids setting on slack-config.yaml admits specific bot IDs. Empty/absent = today's behavior (no other bots).

Changes

forge-plugins/channels/slack/slack.go

  • Plugin struct: new fields ownBotID and allowBotIDs.
  • resolveBotID extended to parse bot_id from the auth.test response alongside the existing user_id.
  • Init parses the comma-separated allow_bot_ids setting via new helper parseAllowBotIDs.
  • New helper admitBotEvent(botID) (reason, admit) encodes the three-rule decision in one testable place. The previous binary filter is replaced by a single call to this helper.
  • Both drop paths emit an operator-actionable log line that names the bot_id and points to slack-config.yaml allow_bot_ids so debugging is self-service.

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

  • Commented example of allow_bot_ids for forge init users, with a pointer to where to find a bot's bot_id in Slack admin and a note that the agent's own messages are always ignored.

Why the allowlist is bot_id rather than app name

bot_id is in every event payload (payload.event.bot_id) without an extra API call, and is stable per-bot-user within a workspace. Resolving an app name to app_id would require a separate apps.info round-trip. Operators can find the bot_id in Slack admin → Manage apps → app → Bot User OAuth.

Loop-safety, recap

Three safeguards keep loops bounded even after admitting some bot mentions:

  1. Self-id guard (unconditional). Encoded in admitBotEvent. The non-negotiable rule from Slack adapter silently drops @-mentions from other bots #55.
  2. The allowlist itself. Bots not in allow_bot_ids stay dropped, so an inadvertent loop requires an explicit operator action.
  3. Mention requirement. The existing block at slack.go:346-361 (untouched) requires <@FORGE_AGENT_USER_ID> in the text even for admitted bots — chatter from an allowed scheduler bot that doesn't tag the agent goes nowhere.

A rate limit on bot-authored invocations is a sensible follow-up if a scheduler misfires, but is not part of this PR.

Tests

gofmt -l forge-core/ forge-cli/ forge-plugins/                       # clean
golangci-lint run ./{forge-core,forge-cli,forge-plugins}/...         # 0 issues
go test ./...     # all three modules pass

New / updated tests in forge-plugins/channels/slack/slack_test.go:

  • TestResolveBotID (updated) — asserts p.ownBotID is captured from the auth.test bot_id field.
  • TestParseAllowBotIDs (table-driven) — empty, single, spaces, empty entries, whitespace-only.
  • TestInit_PopulatesAllowBotIDs — comma-separated allow_bot_ids flows through Init.
  • TestInit_AllowBotIDsAbsent — default behavior preserved when the setting is absent.
  • TestAdmitBotEvent (table-driven) — human admitted, own bot dropped, allowlisted bot admitted, non-allowlisted bot dropped; drop reason includes the allow_bot_ids keyword so operators can grep for it.
  • TestAdmitBotEvent_SelfGuardBeatsAllowlist — explicit regression for the Slack adapter silently drops @-mentions from other bots #55 acceptance criterion that even an own-bot-id-in-allowlist misconfiguration is short-circuited by the self-loop guard.

Acceptance criteria from #55

  • Operator can list bot IDs in slack-config.yaml (allow_bot_ids) and the agent responds to those bots' @-mentions.
  • Default (no setting) preserves today's behavior — every bot-authored event dropped, no surprises.
  • The agent never responds to its own messages, even if its bot_id is in allow_bot_ids (proven by TestAdmitBotEvent_SelfGuardBeatsAllowlist).
  • Admitted bots still must include <@FORGE_AGENT> to trigger — existing mention-matching block at slack.go:346-361 untouched.
  • app_mention events for admitted bots don't double-fire — the Type == "app_mention" dedup at slack.go:341 is unchanged, so the message event remains the single handler path.
  • Operator-actionable log line on every drop, naming the bot_id and pointing to the YAML setting.

Test plan

  • Configure a forge-agent with slack-config.yaml containing no allow_bot_ids. Confirm a human @-mention still triggers a response and a second bot's @-mention is silently ignored (existing behavior preserved).
  • Add a chat:write-capable test bot to the workspace, note its bot_id, add it to allow_bot_ids, restart the agent. Confirm a <@FORGE_AGENT> ping from that bot triggers a response identical to the human case.
  • Confirm an admitted bot posting general chatter (without <@FORGE_AGENT>) does not trigger the agent.
  • Send a message from forge-agent itself (e.g. via its own scheduled output) into a channel where it can read its own message back. Confirm it does not respond — log line names the self-id guard.
  • As a deliberate misconfiguration, put forge-agent's own bot_id in allow_bot_ids. Confirm the agent still ignores its own messages and the log explains why.

…loses #55)

The Slack adapter previously dropped every event with bot_id != "",
blocking legitimate integrations (scheduler, monitoring, workflow,
CI bots @mentioning the agent) while a human posting the same text
would trigger a response. Lifting that filter wholesale would let
the agent respond to its own messages and hot-loop.

This change does two things, both inside the slack adapter:

- Capture the agent's own bot_id at startup. resolveBotID already
  called auth.test to get user_id; the same response payload includes
  bot_id. Store it as p.ownBotID.
- Replace the binary filter with admitBotEvent(botID): humans always
  admitted; the agent's own bot_id is dropped unconditionally (self-
  loop guard, no opt-out, applies even if listed in allow_bot_ids);
  every other bot is admitted only when its bot_id appears in the
  new allow_bot_ids setting on slack-config.yaml.

Default behavior is preserved — with allow_bot_ids absent, no other
bots are admitted. Admitted bots still have to include <@FORGE_AGENT>
in the text to trigger a response (the existing mention-matching
block at slack.go:346-361 is untouched), so an admitted scheduler bot
posting general chatter does not invoke the agent.

Both drop paths emit an operator-actionable log line naming the
bot_id and pointing at the YAML setting.

Tests:
- TestResolveBotID extended: bot_id from auth.test lands on p.ownBotID.
- TestParseAllowBotIDs: empty / single / spaces / empty entries.
- TestInit_PopulatesAllowBotIDs, TestInit_AllowBotIDsAbsent: setting
  flows through Init.
- TestAdmitBotEvent (table-driven): human, own bot, allowlisted,
  non-allowlisted; reason includes "allow_bot_ids".
- TestAdmitBotEvent_SelfGuardBeatsAllowlist: hard rule from #55 —
  even if own bot_id is misconfigured into allow_bot_ids, the
  self-loop guard short-circuits.

Template forge-cli/templates/init/slack-config.yaml.tmpl gets a
commented example of allow_bot_ids so `forge init` users can
discover the option without reading the source.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slack adapter silently drops @-mentions from other bots

1 participant