fix(slack): respect top-level requireMention config
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 <noreply@anthropic.com>
This commit is contained in:
committed by
Peter Steinberger
parent
6ffd7111a6
commit
09ce6ff99e
@@ -75,6 +75,8 @@ export type SlackAccountConfig = {
|
|||||||
appToken?: string;
|
appToken?: string;
|
||||||
/** Allow bot-authored messages to trigger replies (default: false). */
|
/** Allow bot-authored messages to trigger replies (default: false). */
|
||||||
allowBots?: boolean;
|
allowBots?: boolean;
|
||||||
|
/** Default mention requirement for channel messages (default: true). */
|
||||||
|
requireMention?: boolean;
|
||||||
/**
|
/**
|
||||||
* Controls how channel messages are handled:
|
* Controls how channel messages are handled:
|
||||||
* - "open": channels bypass allowlists; mention-gating applies
|
* - "open": channels bypass allowlists; mention-gating applies
|
||||||
|
|||||||
@@ -212,6 +212,7 @@ export const SlackAccountSchema = z.object({
|
|||||||
botToken: z.string().optional(),
|
botToken: z.string().optional(),
|
||||||
appToken: z.string().optional(),
|
appToken: z.string().optional(),
|
||||||
allowBots: z.boolean().optional(),
|
allowBots: z.boolean().optional(),
|
||||||
|
requireMention: z.boolean().optional(),
|
||||||
groupPolicy: GroupPolicySchema.optional().default("allowlist"),
|
groupPolicy: GroupPolicySchema.optional().default("allowlist"),
|
||||||
historyLimit: z.number().int().min(0).optional(),
|
historyLimit: z.number().int().min(0).optional(),
|
||||||
dmHistoryLimit: z.number().int().min(0).optional(),
|
dmHistoryLimit: z.number().int().min(0).optional(),
|
||||||
|
|||||||
@@ -423,6 +423,49 @@ describe("monitorSlackProvider tool results", () => {
|
|||||||
expect(replyMock.mock.calls[0][0].WasMentioned).toBe(true);
|
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 () => {
|
it("treats control commands as mentions for group bypass", async () => {
|
||||||
replyMock.mockResolvedValue({ text: "ok" });
|
replyMock.mockResolvedValue({ text: "ok" });
|
||||||
|
|
||||||
|
|||||||
32
src/slack/monitor/channel-config.test.ts
Normal file
32
src/slack/monitor/channel-config.test.ts
Normal file
@@ -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 });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
@@ -70,8 +70,9 @@ export function resolveSlackChannelConfig(params: {
|
|||||||
systemPrompt?: string;
|
systemPrompt?: string;
|
||||||
}
|
}
|
||||||
>;
|
>;
|
||||||
|
defaultRequireMention?: boolean;
|
||||||
}): SlackChannelConfigResolved | null {
|
}): SlackChannelConfigResolved | null {
|
||||||
const { channelId, channelName, channels } = params;
|
const { channelId, channelName, channels, defaultRequireMention } = params;
|
||||||
const entries = channels ?? {};
|
const entries = channels ?? {};
|
||||||
const keys = Object.keys(entries);
|
const keys = Object.keys(entries);
|
||||||
const normalizedName = channelName ? normalizeSlackSlug(channelName) : "";
|
const normalizedName = channelName ? normalizeSlackSlug(channelName) : "";
|
||||||
@@ -102,11 +103,12 @@ export function resolveSlackChannelConfig(params: {
|
|||||||
}
|
}
|
||||||
const fallback = entries["*"];
|
const fallback = entries["*"];
|
||||||
|
|
||||||
|
const requireMentionDefault = defaultRequireMention ?? true;
|
||||||
if (keys.length === 0) {
|
if (keys.length === 0) {
|
||||||
return { allowed: true, requireMention: true };
|
return { allowed: true, requireMention: requireMentionDefault };
|
||||||
}
|
}
|
||||||
if (!matched && !fallback) {
|
if (!matched && !fallback) {
|
||||||
return { allowed: false, requireMention: true };
|
return { allowed: false, requireMention: requireMentionDefault };
|
||||||
}
|
}
|
||||||
|
|
||||||
const resolved = matched ?? fallback ?? {};
|
const resolved = matched ?? fallback ?? {};
|
||||||
@@ -114,7 +116,8 @@ export function resolveSlackChannelConfig(params: {
|
|||||||
firstDefined(resolved.enabled, resolved.allow, fallback?.enabled, fallback?.allow, true) ??
|
firstDefined(resolved.enabled, resolved.allow, fallback?.enabled, fallback?.allow, true) ??
|
||||||
true;
|
true;
|
||||||
const requireMention =
|
const requireMention =
|
||||||
firstDefined(resolved.requireMention, fallback?.requireMention, true) ?? true;
|
firstDefined(resolved.requireMention, fallback?.requireMention, requireMentionDefault) ??
|
||||||
|
requireMentionDefault;
|
||||||
const allowBots = firstDefined(resolved.allowBots, fallback?.allowBots);
|
const allowBots = firstDefined(resolved.allowBots, fallback?.allowBots);
|
||||||
const users = firstDefined(resolved.users, fallback?.users);
|
const users = firstDefined(resolved.users, fallback?.users);
|
||||||
const skills = firstDefined(resolved.skills, fallback?.skills);
|
const skills = firstDefined(resolved.skills, fallback?.skills);
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ export type SlackMonitorContext = {
|
|||||||
allowFrom: string[];
|
allowFrom: string[];
|
||||||
groupDmEnabled: boolean;
|
groupDmEnabled: boolean;
|
||||||
groupDmChannels: string[];
|
groupDmChannels: string[];
|
||||||
|
defaultRequireMention: boolean;
|
||||||
channelsConfig?: Record<
|
channelsConfig?: Record<
|
||||||
string,
|
string,
|
||||||
{
|
{
|
||||||
@@ -103,6 +104,7 @@ export function createSlackMonitorContext(params: {
|
|||||||
allowFrom: Array<string | number> | undefined;
|
allowFrom: Array<string | number> | undefined;
|
||||||
groupDmEnabled: boolean;
|
groupDmEnabled: boolean;
|
||||||
groupDmChannels: Array<string | number> | undefined;
|
groupDmChannels: Array<string | number> | undefined;
|
||||||
|
defaultRequireMention?: boolean;
|
||||||
channelsConfig?: SlackMonitorContext["channelsConfig"];
|
channelsConfig?: SlackMonitorContext["channelsConfig"];
|
||||||
groupPolicy: SlackMonitorContext["groupPolicy"];
|
groupPolicy: SlackMonitorContext["groupPolicy"];
|
||||||
useAccessGroups: boolean;
|
useAccessGroups: boolean;
|
||||||
@@ -132,6 +134,7 @@ export function createSlackMonitorContext(params: {
|
|||||||
|
|
||||||
const allowFrom = normalizeAllowList(params.allowFrom);
|
const allowFrom = normalizeAllowList(params.allowFrom);
|
||||||
const groupDmChannels = normalizeAllowList(params.groupDmChannels);
|
const groupDmChannels = normalizeAllowList(params.groupDmChannels);
|
||||||
|
const defaultRequireMention = params.defaultRequireMention ?? true;
|
||||||
|
|
||||||
const markMessageSeen = (channelId: string | undefined, ts?: string) => {
|
const markMessageSeen = (channelId: string | undefined, ts?: string) => {
|
||||||
if (!channelId || !ts) return false;
|
if (!channelId || !ts) return false;
|
||||||
@@ -274,6 +277,7 @@ export function createSlackMonitorContext(params: {
|
|||||||
channelId: p.channelId,
|
channelId: p.channelId,
|
||||||
channelName: p.channelName,
|
channelName: p.channelName,
|
||||||
channels: params.channelsConfig,
|
channels: params.channelsConfig,
|
||||||
|
defaultRequireMention,
|
||||||
});
|
});
|
||||||
const channelAllowed = channelConfig?.allowed !== false;
|
const channelAllowed = channelConfig?.allowed !== false;
|
||||||
const channelAllowlistConfigured =
|
const channelAllowlistConfigured =
|
||||||
@@ -330,6 +334,7 @@ export function createSlackMonitorContext(params: {
|
|||||||
allowFrom,
|
allowFrom,
|
||||||
groupDmEnabled: params.groupDmEnabled,
|
groupDmEnabled: params.groupDmEnabled,
|
||||||
groupDmChannels,
|
groupDmChannels,
|
||||||
|
defaultRequireMention,
|
||||||
channelsConfig: params.channelsConfig,
|
channelsConfig: params.channelsConfig,
|
||||||
groupPolicy: params.groupPolicy,
|
groupPolicy: params.groupPolicy,
|
||||||
useAccessGroups: params.useAccessGroups,
|
useAccessGroups: params.useAccessGroups,
|
||||||
|
|||||||
@@ -56,6 +56,7 @@ export async function prepareSlackMessage(params: {
|
|||||||
channelId: message.channel,
|
channelId: message.channel,
|
||||||
channelName,
|
channelName,
|
||||||
channels: ctx.channelsConfig,
|
channels: ctx.channelsConfig,
|
||||||
|
defaultRequireMention: ctx.defaultRequireMention,
|
||||||
})
|
})
|
||||||
: null;
|
: null;
|
||||||
|
|
||||||
@@ -200,7 +201,9 @@ export async function prepareSlackMessage(params: {
|
|||||||
cfg,
|
cfg,
|
||||||
surface: "slack",
|
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.
|
// Allow "control commands" to bypass mention gating if sender is authorized.
|
||||||
const shouldBypassMention =
|
const shouldBypassMention =
|
||||||
|
|||||||
@@ -122,6 +122,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
|
|||||||
allowFrom,
|
allowFrom,
|
||||||
groupDmEnabled,
|
groupDmEnabled,
|
||||||
groupDmChannels,
|
groupDmChannels,
|
||||||
|
defaultRequireMention: slackCfg.requireMention,
|
||||||
channelsConfig,
|
channelsConfig,
|
||||||
groupPolicy,
|
groupPolicy,
|
||||||
useAccessGroups,
|
useAccessGroups,
|
||||||
|
|||||||
@@ -136,6 +136,7 @@ export function registerSlackMonitorSlashCommands(params: {
|
|||||||
channelId: command.channel_id,
|
channelId: command.channel_id,
|
||||||
channelName: channelInfo?.name,
|
channelName: channelInfo?.name,
|
||||||
channels: ctx.channelsConfig,
|
channels: ctx.channelsConfig,
|
||||||
|
defaultRequireMention: ctx.defaultRequireMention,
|
||||||
});
|
});
|
||||||
if (ctx.useAccessGroups) {
|
if (ctx.useAccessGroups) {
|
||||||
const channelAllowlistConfigured =
|
const channelAllowlistConfigured =
|
||||||
|
|||||||
Reference in New Issue
Block a user