From 09ce6ff99e9b7f757cfd9031850c05ae30f487be Mon Sep 17 00:00:00 2001 From: Jonathan Wilkins Date: Tue, 13 Jan 2026 14:21:23 +0000 Subject: [PATCH] fix(slack): respect top-level requireMention config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `channels.slack.requireMention` setting was defined in the schema but never passed to `resolveSlackChannelConfig()`, which always defaulted to `true`. This meant setting `requireMention: false` at the top level had no effect—channels still required mentions. Pass `slackCfg.requireMention` as `defaultRequireMention` to the resolver and use it as the fallback instead of hardcoded `true`. Co-Authored-By: Claude Opus 4.5 --- src/config/types.slack.ts | 2 + src/config/zod-schema.providers-core.ts | 1 + ...ends-tool-summaries-responseprefix.test.ts | 43 +++++++++++++++++++ src/slack/monitor/channel-config.test.ts | 32 ++++++++++++++ src/slack/monitor/channel-config.ts | 11 +++-- src/slack/monitor/context.ts | 5 +++ src/slack/monitor/message-handler/prepare.ts | 5 ++- src/slack/monitor/provider.ts | 1 + src/slack/monitor/slash.ts | 1 + 9 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 src/slack/monitor/channel-config.test.ts diff --git a/src/config/types.slack.ts b/src/config/types.slack.ts index 16f6c6d10..20ed3f8ce 100644 --- a/src/config/types.slack.ts +++ b/src/config/types.slack.ts @@ -75,6 +75,8 @@ export type SlackAccountConfig = { appToken?: string; /** Allow bot-authored messages to trigger replies (default: false). */ allowBots?: boolean; + /** Default mention requirement for channel messages (default: true). */ + requireMention?: boolean; /** * Controls how channel messages are handled: * - "open": channels bypass allowlists; mention-gating applies diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index 64ad7286b..00bd547de 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -212,6 +212,7 @@ export const SlackAccountSchema = z.object({ botToken: z.string().optional(), appToken: z.string().optional(), allowBots: z.boolean().optional(), + requireMention: z.boolean().optional(), groupPolicy: GroupPolicySchema.optional().default("allowlist"), historyLimit: z.number().int().min(0).optional(), dmHistoryLimit: z.number().int().min(0).optional(), diff --git a/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts b/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts index 01e36052b..400d84bb2 100644 --- a/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts +++ b/src/slack/monitor.tool-result.sends-tool-summaries-responseprefix.test.ts @@ -423,6 +423,49 @@ describe("monitorSlackProvider tool results", () => { expect(replyMock.mock.calls[0][0].WasMentioned).toBe(true); }); + it("accepts channel messages without mention when channels.slack.requireMention is false", async () => { + config = { + channels: { + slack: { + dm: { enabled: true, policy: "open", allowFrom: ["*"] }, + groupPolicy: "open", + requireMention: false, + }, + }, + }; + replyMock.mockResolvedValue({ text: "hi" }); + + const controller = new AbortController(); + const run = monitorSlackProvider({ + botToken: "bot-token", + appToken: "app-token", + abortSignal: controller.signal, + }); + + await waitForEvent("message"); + const handler = getSlackHandlers()?.get("message"); + if (!handler) throw new Error("Slack message handler not registered"); + + await handler({ + event: { + type: "message", + user: "U1", + text: "hello", + ts: "123", + channel: "C1", + channel_type: "channel", + }, + }); + + await flush(); + controller.abort(); + await run; + + expect(replyMock).toHaveBeenCalledTimes(1); + expect(replyMock.mock.calls[0][0].WasMentioned).toBe(false); + expect(sendMock).toHaveBeenCalledTimes(1); + }); + it("treats control commands as mentions for group bypass", async () => { replyMock.mockResolvedValue({ text: "ok" }); diff --git a/src/slack/monitor/channel-config.test.ts b/src/slack/monitor/channel-config.test.ts new file mode 100644 index 000000000..60d704d97 --- /dev/null +++ b/src/slack/monitor/channel-config.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; + +import { resolveSlackChannelConfig } from "./channel-config.js"; + +describe("resolveSlackChannelConfig", () => { + it("uses defaultRequireMention when channels config is empty", () => { + const res = resolveSlackChannelConfig({ + channelId: "C1", + channels: {}, + defaultRequireMention: false, + }); + expect(res).toEqual({ allowed: true, requireMention: false }); + }); + + it("defaults defaultRequireMention to true when not provided", () => { + const res = resolveSlackChannelConfig({ + channelId: "C1", + channels: {}, + }); + expect(res).toEqual({ allowed: true, requireMention: true }); + }); + + it("prefers explicit channel/fallback requireMention over defaultRequireMention", () => { + const res = resolveSlackChannelConfig({ + channelId: "C1", + channels: { "*": { requireMention: true } }, + defaultRequireMention: false, + }); + expect(res).toMatchObject({ requireMention: true }); + }); +}); + diff --git a/src/slack/monitor/channel-config.ts b/src/slack/monitor/channel-config.ts index e97adc876..712371d98 100644 --- a/src/slack/monitor/channel-config.ts +++ b/src/slack/monitor/channel-config.ts @@ -70,8 +70,9 @@ export function resolveSlackChannelConfig(params: { systemPrompt?: string; } >; + defaultRequireMention?: boolean; }): SlackChannelConfigResolved | null { - const { channelId, channelName, channels } = params; + const { channelId, channelName, channels, defaultRequireMention } = params; const entries = channels ?? {}; const keys = Object.keys(entries); const normalizedName = channelName ? normalizeSlackSlug(channelName) : ""; @@ -102,11 +103,12 @@ export function resolveSlackChannelConfig(params: { } const fallback = entries["*"]; + const requireMentionDefault = defaultRequireMention ?? true; if (keys.length === 0) { - return { allowed: true, requireMention: true }; + return { allowed: true, requireMention: requireMentionDefault }; } if (!matched && !fallback) { - return { allowed: false, requireMention: true }; + return { allowed: false, requireMention: requireMentionDefault }; } const resolved = matched ?? fallback ?? {}; @@ -114,7 +116,8 @@ export function resolveSlackChannelConfig(params: { firstDefined(resolved.enabled, resolved.allow, fallback?.enabled, fallback?.allow, true) ?? true; const requireMention = - firstDefined(resolved.requireMention, fallback?.requireMention, true) ?? true; + firstDefined(resolved.requireMention, fallback?.requireMention, requireMentionDefault) ?? + requireMentionDefault; const allowBots = firstDefined(resolved.allowBots, fallback?.allowBots); const users = firstDefined(resolved.users, fallback?.users); const skills = firstDefined(resolved.skills, fallback?.skills); diff --git a/src/slack/monitor/context.ts b/src/slack/monitor/context.ts index 5f96f68af..19ce39fc0 100644 --- a/src/slack/monitor/context.ts +++ b/src/slack/monitor/context.ts @@ -34,6 +34,7 @@ export type SlackMonitorContext = { allowFrom: string[]; groupDmEnabled: boolean; groupDmChannels: string[]; + defaultRequireMention: boolean; channelsConfig?: Record< string, { @@ -103,6 +104,7 @@ export function createSlackMonitorContext(params: { allowFrom: Array | undefined; groupDmEnabled: boolean; groupDmChannels: Array | undefined; + defaultRequireMention?: boolean; channelsConfig?: SlackMonitorContext["channelsConfig"]; groupPolicy: SlackMonitorContext["groupPolicy"]; useAccessGroups: boolean; @@ -132,6 +134,7 @@ export function createSlackMonitorContext(params: { const allowFrom = normalizeAllowList(params.allowFrom); const groupDmChannels = normalizeAllowList(params.groupDmChannels); + const defaultRequireMention = params.defaultRequireMention ?? true; const markMessageSeen = (channelId: string | undefined, ts?: string) => { if (!channelId || !ts) return false; @@ -274,6 +277,7 @@ export function createSlackMonitorContext(params: { channelId: p.channelId, channelName: p.channelName, channels: params.channelsConfig, + defaultRequireMention, }); const channelAllowed = channelConfig?.allowed !== false; const channelAllowlistConfigured = @@ -330,6 +334,7 @@ export function createSlackMonitorContext(params: { allowFrom, groupDmEnabled: params.groupDmEnabled, groupDmChannels, + defaultRequireMention, channelsConfig: params.channelsConfig, groupPolicy: params.groupPolicy, useAccessGroups: params.useAccessGroups, diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index fd89400ce..224426823 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -56,6 +56,7 @@ export async function prepareSlackMessage(params: { channelId: message.channel, channelName, channels: ctx.channelsConfig, + defaultRequireMention: ctx.defaultRequireMention, }) : null; @@ -200,7 +201,9 @@ export async function prepareSlackMessage(params: { cfg, surface: "slack", }); - const shouldRequireMention = isRoom ? (channelConfig?.requireMention ?? true) : false; + const shouldRequireMention = isRoom + ? (channelConfig?.requireMention ?? ctx.defaultRequireMention) + : false; // Allow "control commands" to bypass mention gating if sender is authorized. const shouldBypassMention = diff --git a/src/slack/monitor/provider.ts b/src/slack/monitor/provider.ts index a0a46e949..53a67f0f4 100644 --- a/src/slack/monitor/provider.ts +++ b/src/slack/monitor/provider.ts @@ -122,6 +122,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { allowFrom, groupDmEnabled, groupDmChannels, + defaultRequireMention: slackCfg.requireMention, channelsConfig, groupPolicy, useAccessGroups, diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index 1c1af9477..0dab949ca 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -136,6 +136,7 @@ export function registerSlackMonitorSlashCommands(params: { channelId: command.channel_id, channelName: channelInfo?.name, channels: ctx.channelsConfig, + defaultRequireMention: ctx.defaultRequireMention, }); if (ctx.useAccessGroups) { const channelAllowlistConfigured =