fix(slack): require mentions only for channel roots (#20821)

## Thinking Path

> - Paperclip helps people manage AI agents for work.
> - Slack connects people to those agents through channels and threads.
> - A mention can start a conversation without making the bot answer all
channel traffic.
> - The current setting also requires mentions on every thread reply.
> - This interrupts an existing conversation and can cancel a queued
reply.
> - This change requires mentions on channel roots and allows replies
without a mention.
> - People can continue a thread without adding the bot name to each
message.

## Linked Issues or Issue Description

**What existing behavior does this improve?**
The Slack connection setting that requires an at-mention.

**Current behavior**
When the setting is on, a channel thread reply must mention the bot. The
delivery and execution checks reject a reply that has no mention.

**Proposed behavior**
Require a mention on a top-level channel message. Allow replies in
existing threads without a mention. Keep the current DM access controls.

**Reason and benefit**
A person can continue an existing conversation without repeating the bot
name.

**Breaking changes**
Existing connections with the setting on will accept thread replies
without a mention. The saved setting, default value, and API field stay
the same.

## What Changed

- Rename the control to **Require at-mention in channels**. Explain the
thread and DM rules in the help text and operator documentation.
- Keep the mention policy in the Slack module. Use the thread root and
message IDs to identify a reply.
- Call that policy from durable admission, wake authorization, and
queued run start. These shared paths must enforce the same setting when
a delivery resumes. Other providers keep their current behavior.
- Update PostgreSQL integration tests for thread replies, DMs, queued
deliveries, and combined wake inputs. Add tests that reject incomplete
reply identity.

## Verification

- Reproduced rejected thread replies and cancelled queued reply runs
before the fix.
- Passed 16 focused PostgreSQL integration cases after the fix.
- Passed 71 tests on revision `31b1adede2b92bad24b8fa21b8f6fffa9bd26d04`
after the rebase. This includes the full durable wakeup suite, Slack
policy tests, and eight selected Slack delivery cases.
- Command: `pnpm exec vitest run
server/src/services/chat-channels/slack/wake-authority.test.ts
server/src/__tests__/durable-chat-wakeup.test.ts
server/src/__tests__/chat-channels.integration.test.ts -t "at-mentions
by default|thread replies without mentions|rechecks queued
Slack|preserves running Slack tool authority|Slack channel mention
policy|durable inbound chat scheduler receipts"`.
- Strengthened the queued-root test to check the mention-rejection
reason and content redaction. A temporary change that disabled mention
enforcement caused this test to fail with the unrelated-message reason.
The real policy is restored.
- Removed the added source comments to follow the contribution rules.
- `node scripts/check-module-boundaries.mjs` and `git diff --check`
pass.
- `pnpm verify` returns exit 2 because the operator documentation is a
global CI input. Required CI must supply the wider coverage.
- Local preflight installed dependencies and built the plugin SDK. It
could not build Runner Rust binaries because this environment has no
`cargo` executable. The focused server tests run without those binaries.
- Live Slack and browser tests were not run. This change uses the
existing provider message and thread identities.

- Required CI passed on `31b1adede2b92bad24b8fa21b8f6fffa9bd26d04`: [PR
workflow](https://github.com/paperclipai/paperclip/actions/runs/43035551294),
including typecheck, build, server/chat tests, Runner checks, and
browser E2E. Current-head Greptile and Paperclip reviews are both 5/5
with no open findings.

## Risks

- Existing connections with mention gating on will now accept
unmentioned thread replies. This is the intended behavior change.
- A missing or invalid thread or message ID does not grant the reply
exception.
- No schema migration is required. Destination, sender, and company
access checks stay in place.
- The existing chat integration test file is large. This change updates
its current Slack cases to reuse the full delivery and authorization
fixture.

## Model Used

OpenAI Codex, based on GPT-6, with reasoning and code execution tools.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed 2026-11-11 08:26:52 -06:00
1 parent fa3cd6f330
commit aaf1337c7b
9 files changed
+143 -50

No files matched your search

@@ -11666,7 +11666,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
await expect(resolveSlackTaskAuthority(db, binding)).resolves.toMatchObject({ slackUserId: "UBOARD" });
});
it.each([false, true])("applies Slack mention settings to channel replies while exempting DMs (DM=%s)", async (isDM) => {
it.each([false, true])("allows Slack thread replies without mentions while exempting DMs (DM=%s)", async (isDM) => {
const fixture = await seedCompany();
const { callbacks, endpoint, service, wakeup } = await configuredSlackEndpoint(fixture);
expect((await service.get(endpoint.id)).requireAtMention).toBe(false);
@@ -11680,29 +11680,23 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
expect(wakeup).toHaveBeenCalledTimes(2);
expect((await service.update(endpoint.id, { requireAtMention: true })).requireAtMention).toBe(true);
await send("7100.3", false, "subscribed_message");
expect(wakeup).toHaveBeenCalledTimes(isDM ? 3 : 2);
expect(wakeup).toHaveBeenCalledTimes(3);
const [filtered] = await db.select().from(chatDeliveries).where(and(
eq(chatDeliveries.endpointId, endpoint.id), eq(chatDeliveries.state, "filtered")));
if (isDM) {
expect(filtered).toBeUndefined();
} else {
expect(filtered.redactedError).toBe("Message did not @mention the bot");
expect(filtered.normalizedEvent).toMatchObject({ filtering: { contentRetained: false } });
expect(filtered.normalizedEvent).not.toHaveProperty("message.text");
}
expect(filtered).toBeUndefined();
// Both Slack callback kinds can carry an explicit mention. Retries cannot
// produce a second wakeup for the same provider message.
await send("7100.4", true);
await send("7100.4", true, "mention");
expect(wakeup).toHaveBeenCalledTimes(isDM ? 4 : 3);
expect(wakeup).toHaveBeenCalledTimes(4);
const conversations = await readChatConversationsByEndpointId(endpoint.id);
expect(conversations).toHaveLength(1);
const comments = await readIssueCommentsByIssueId(conversations[0].issueId);
expect(comments).toHaveLength(isDM ? 4 : 3);
expect(comments).toHaveLength(4);
expect(conversations[0].externalThreadId).toBe(thread.thread.id);
await service.update(endpoint.id, { requireAtMention: false });
await send("7100.5", false);
expect(wakeup).toHaveBeenCalledTimes(isDM ? 5 : 4);
expect(wakeup).toHaveBeenCalledTimes(5);
if (isDM) {
await service.update(endpoint.id, { requireAtMention: true, allowDirectMessages: false });
await send("7100.6", false);
@@ -11710,7 +11704,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
}
});
it.each([false, true])("rechecks queued Slack channel mentions but exempts DMs before drain (DM=%s)", async (isDM) => {
it.each([
{ isDM: false, isReply: false },
{ isDM: false, isReply: true },
{ isDM: true, isReply: false },
])("rechecks queued Slack channel roots but exempts replies and DMs (DM=$isDM, reply=$isReply)", async ({ isDM, isReply }) => {
const fixture = await seedCompany();
const { callbacks, endpoint, service, wakeup } = await configuredSlackEndpoint(fixture, {
deferWebhookProcessing: true, scheduleDeferredWork: () => undefined,
@@ -11718,14 +11716,28 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
const channelId = isDM ? "D-QUEUED-MENTION" : "C-QUEUED-MENTION";
const thread = makeThread({ channelId, id: `slack:${channelId}:${isDM ? "" : "7300.1"}`, isDM });
await deliverMessage({ callbacks, endpointId: endpoint.id, thread: thread.thread,
message: makeMessage({ id: "7300.1", text: "Start work" }), trigger: isDM ? "direct_message" : "subscribed_message" });
message: makeMessage({ id: "7300.1", text: "@maya start", mentioned: true }), trigger: isDM ? "direct_message" : "mention" });
const [starter] = await readChatDeliveriesByEndpointId(endpoint.id);
await db.update(chatDeliveries).set({ nextAttemptAt: new Date(0) }).where(eq(chatDeliveries.id, starter.id));
await service.processPendingDeliveries(25, starter.id);
expect(wakeup).toHaveBeenCalledTimes(1);
const nextThread = isReply || isDM ? thread.thread : makeThread({ channelId, id: `slack:${channelId}:7300.2` }).thread;
await deliverMessage({ callbacks, endpointId: endpoint.id, thread: nextThread,
message: makeMessage({ id: "7300.2", text: "Continue work" }), trigger: isDM ? "direct_message" : "subscribed_message" });
await service.update(endpoint.id, { requireAtMention: true });
const [delivery] = await readChatDeliveriesByEndpointId(endpoint.id);
const delivery = (await readChatDeliveriesByEndpointId(endpoint.id)).find(row => row.id !== starter.id)!;
await db.update(chatDeliveries).set({ nextAttemptAt: new Date(0) }).where(eq(chatDeliveries.id, delivery.id));
await service.processPendingDeliveries(25, delivery.id);
expect(wakeup).toHaveBeenCalledTimes(isDM ? 1 : 0);
expect(await readChatConversationsByEndpointId(endpoint.id)).toHaveLength(isDM ? 1 : 0);
expect((await readChatDeliveriesById(delivery.id))[0].state).toBe(isDM ? "processed" : "filtered");
const allowed = isDM || isReply;
expect(wakeup).toHaveBeenCalledTimes(allowed ? 2 : 1);
expect(await readChatConversationsByEndpointId(endpoint.id)).toHaveLength(1);
const [result] = await readChatDeliveriesById(delivery.id);
expect(result.state).toBe(allowed ? "processed" : "filtered");
if (!allowed) {
expect(result.redactedError).toBe("Message did not @mention the bot");
expect(result.normalizedEvent).toMatchObject({ filtering: { contentRetained: false } });
expect(result.normalizedEvent).not.toHaveProperty("message.text");
}
});
it("snapshots channel workspace defaults on new tasks and preserves them on replies", async () => {
@@ -197,11 +197,11 @@ describe("durable inbound chat scheduler receipts", () => {
}
it.each([
...["unmentioned", "mentioned", "coalesced", "recovery", "deferred"].map((scenario) => ({ scenario, isDM: false })),
...["unmentioned", "mentioned", "reply", "coalesced_reply", "coalesced", "recovery", "deferred"].map((scenario) => ({ scenario, isDM: false })),
...["unmentioned", "coalesced"].map((scenario) => ({ scenario, isDM: true })),
])("checks Slack channel mentions after a real heartbeat queue wait ($scenario, DM=$isDM)", async ({ scenario, isDM }) => {
const mentioned = scenario === "mentioned" || scenario === "coalesced";
const allowed = isDM || scenario === "mentioned" || scenario === "recovery";
const allowed = isDM || scenario === "mentioned" || scenario === "recovery" || scenario === "reply" || scenario === "coalesced_reply";
const f = await fixture();
await db.update(agents).set({ adapterType: "durable_chat_retry_test" }).where(eq(agents.id, f.agentId));
const applicationId = randomUUID(), connectionId = randomUUID(), endpointId = randomUUID();
@@ -215,7 +215,7 @@ describe("durable inbound chat scheduler receipts", () => {
authorUserId: "board-user", body: mentioned ? "@bot work" : "Work" });
const [delivery] = await db.insert(chatDeliveries).values({ companyId: f.companyId, endpointId,
providerEventId: randomUUID(), deduplicationKey: randomUUID(), eventKind: isDM ? "direct_message" : "message", state: "processed",
normalizedEvent: { trigger: isDM ? "direct_message" : "subscribed_message", conversation: { isDirectMessage: isDM }, message: { mentionedBot: mentioned } } }).returning();
normalizedEvent: { trigger: isDM ? "direct_message" : "subscribed_message", conversation: { isDirectMessage: isDM, externalThreadId: "slack:C-MENTION:7300.1" }, message: { providerMessageId: scenario === "reply" || scenario === "coalesced_reply" ? "7300.2" : "7300.1", mentionedBot: mentioned } } }).returning();
await db.insert(chatActions).values({ id: request.id, companyId: f.companyId, endpointId,
deliveryId: delivery.id, kind: "inbound_wakeup", providerActionId: randomUUID(), status: "processed",
payload: { issueId: f.issueId, commentId: request.commentId } });
@@ -231,13 +231,13 @@ describe("durable inbound chat scheduler receipts", () => {
.where(eq(heartbeatRuns.id, receipt.runId!));
}
let deferredRequestId: string | undefined;
if (scenario === "coalesced" || scenario === "deferred") {
if (scenario === "coalesced" || scenario === "coalesced_reply" || scenario === "deferred") {
const second = f.request();
await db.insert(issueComments).values({ id: second.commentId, companyId: f.companyId, issueId: f.issueId,
authorUserId: "board-user", body: scenario === "deferred" ? "@bot follow-up" : "Unmentioned follow-up" });
const [secondDelivery] = await db.insert(chatDeliveries).values({ companyId: f.companyId, endpointId,
providerEventId: randomUUID(), deduplicationKey: randomUUID(), eventKind: isDM ? "direct_message" : "message", state: "processed",
normalizedEvent: { trigger: isDM ? "direct_message" : "subscribed_message", conversation: { isDirectMessage: isDM }, message: { mentionedBot: scenario === "deferred" } } }).returning();
normalizedEvent: { trigger: isDM ? "direct_message" : "subscribed_message", conversation: { isDirectMessage: isDM, externalThreadId: "slack:C-MENTION:7300.1" }, message: { providerMessageId: scenario === "coalesced_reply" ? "7300.3" : "7300.1", mentionedBot: scenario === "deferred" } } }).returning();
await db.insert(chatActions).values({ id: second.id, companyId: f.companyId, endpointId,
deliveryId: secondDelivery.id, kind: "inbound_wakeup", providerActionId: randomUUID(), status: "processed",
payload: { issueId: f.issueId, commentId: second.commentId } });
@@ -1,3 +1,4 @@
import { slackMessageMeetsMentionRequirement } from "../slack/messages.js";
import { createTeamsDestinationAdmission } from "../microsoft-teams/destination-admission.js";
import { createTelegramDestinationAdmission } from "../telegram/destination-admission.js";
import { createPhotonDestinationAdmission } from "../imessage-photon/destination-admission.js";
@@ -167,10 +168,7 @@ export function createInboundAdmission(db: Db, options: InboundAdmissionOptions)
}
const mentionRequired =
endpoint.provider === "slack" &&
currentEndpoint.requireAtMention &&
!thread.isDM &&
trigger !== "mention" &&
message.isMention !== true;
!slackMessageMeetsMentionRequirement(currentEndpoint, thread, trigger, message);
// GitHub sends the same comment to every installed App. Subscribed
// conversations are a fallback for unaddressed replies, not permission
// to wake every bot when the person addresses a different recipient.
@@ -106,9 +106,15 @@ export function stripSlackTitleMentions(text: string): string {
export function slackMessageMeetsMentionRequirement(
endpoint: Pick<EndpointRow, "requireAtMention">,
thread: Pick<Thread, "isDM">,
thread: Pick<Thread, "id" | "isDM">,
trigger: ChatSdkMessageCallbackEvent["trigger"],
message: Pick<Message, "isMention">,
message: Pick<Message, "id" | "isMention">,
): boolean {
return !endpoint.requireAtMention || thread.isDM || trigger === "mention" || message.isMention === true;
return (
!endpoint.requireAtMention ||
thread.isDM ||
(Boolean(message.id) && slackOrphanFollowUp(thread.id, message.id)) ||
trigger === "mention" ||
message.isMention === true
);
}
@@ -0,0 +1,48 @@
import { describe, expect, it } from "vitest";
import { slackDeliveryMeetsMentionRequirement } from "./wake-authority.js";
describe("Slack channel mention policy", () => {
const root = {
trigger: "subscribed_message",
conversation: { externalThreadId: "slack:C123:7300.000001", isDirectMessage: false },
message: { providerMessageId: "7300.000001", mentionedBot: false },
};
it("requires a mention on a channel root, even for a subscribed callback", () => {
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, root)).toBe(false);
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: false }, root)).toBe(true);
});
it("allows replies with a known message ID in a channel thread", () => {
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, {
...root, message: { ...root.message, providerMessageId: "7300.000002" },
})).toBe(true);
});
it.each([
{},
{ ...root, message: {} },
{ ...root, message: { providerMessageId: "" } },
{ ...root, message: { providerMessageId: 7300 } },
{ ...root, conversation: {} },
{ ...root, conversation: { externalThreadId: "slack:C123:" } },
{ ...root, conversation: { externalThreadId: "slack:C123" } },
{ ...root, conversation: { externalThreadId: "discord:C123:7300.000001" } },
])("does not infer a thread reply from incomplete or invalid identity (%j)", (event) => {
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, event)).toBe(false);
});
it("accepts either explicit mention callback", () => {
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, { ...root, trigger: "mention" })).toBe(true);
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, {
...root, message: { ...root.message, mentionedBot: true },
})).toBe(true);
});
it("exempts DMs and uses the current conversation type when it is supplied", () => {
const directMessage = { ...root, conversation: { ...root.conversation, isDirectMessage: true } };
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, directMessage)).toBe(true);
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, root, true)).toBe(true);
expect(slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, directMessage, false)).toBe(false);
});
});
@@ -1,14 +1,44 @@
import { forbidden } from "../../../errors.js";
import { slackMessageMeetsMentionRequirement } from "./messages.js";
import type { EndpointRow } from "../runtime-types.js";
import type { chatConversations, chatDeliveries } from "@paperclipai/db";
export function assertSlackWakeMention(endpoint: EndpointRow, conversation: Pick<typeof chatConversations.$inferSelect, "isDirectMessage">, delivery: Pick<typeof chatDeliveries.$inferSelect, "normalizedEvent">): void {
if (
endpoint.requireAtMention &&
!conversation.isDirectMessage &&
delivery.normalizedEvent.trigger !== "mention" &&
(delivery.normalizedEvent.message as { mentionedBot?: boolean } | undefined)?.mentionedBot !== true
) {
type SlackDeliveryEvent = typeof chatDeliveries.$inferSelect["normalizedEvent"];
export function slackDeliveryMeetsMentionRequirement(
endpoint: Pick<EndpointRow, "requireAtMention">,
event: SlackDeliveryEvent,
isDirectMessage?: boolean,
): boolean {
const conversation = event.conversation as {
externalThreadId?: unknown;
isDirectMessage?: boolean;
} | undefined;
const message = event.message as {
providerMessageId?: unknown;
mentionedBot?: boolean;
} | undefined;
return slackMessageMeetsMentionRequirement(
endpoint,
{
id: typeof conversation?.externalThreadId === "string" ? conversation.externalThreadId : "",
isDM: isDirectMessage ?? conversation?.isDirectMessage === true,
},
event.trigger === "mention" ? "mention" : "subscribed_message",
{
id: typeof message?.providerMessageId === "string" ? message.providerMessageId : "",
isMention: message?.mentionedBot === true,
},
);
}
export function assertSlackWakeMention(
endpoint: EndpointRow,
conversation: Pick<typeof chatConversations.$inferSelect, "isDirectMessage">,
delivery: Pick<typeof chatDeliveries.$inferSelect, "normalizedEvent">,
): void {
if (!slackDeliveryMeetsMentionRequirement(endpoint, delivery.normalizedEvent, conversation.isDirectMessage)) {
throw forbidden("Message did not @mention the bot", {
code: "chat_action_authorization_changed",
});
+2 -3
View File
@@ -1,3 +1,4 @@
import { slackDeliveryMeetsMentionRequirement } from "./chat-channels/slack/wake-authority.js";
import { createHash } from "node:crypto";
import { agentWakeupRequests, chatActions, chatConversations, chatDeliveries, chatEndpoints, type Db } from "@paperclipai/db";
import { and, eq, exists, inArray, or, sql, type SQLWrapper } from "drizzle-orm";
@@ -410,7 +411,5 @@ export async function slackMentionAllowsRunStart(
),
));
return inputs.every(({ event }) =>
(event.conversation as { isDirectMessage?: boolean } | undefined)?.isDirectMessage === true ||
event.trigger === "mention" ||
(event.message as { mentionedBot?: boolean } | undefined)?.mentionedBot === true);
slackDeliveryMeetsMentionRequirement({ requireAtMention: true }, event));
}