From 4a4129266ead6f3af3e8fa2992a75f554267e5a9 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Tue, 1 Sep 2026 15:17:44 +0900 Subject: [PATCH] fix(discordbot): ignore replies addressed to other users --- services/discordbot/README.md | 5 +- services/discordbot/src/discord-ingress.ts | 22 +++++++- .../discordbot/src/discord-mention-routing.ts | 33 ++++++++++++ .../discordbot/test/chat-sdk-emulate.test.ts | 53 ++++++++++++++++++- .../discordbot/test/discord-ingress.test.ts | 53 +++++++++++++++++++ .../test/discord-mention-routing.test.ts | 44 +++++++++++++++ 6 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 services/discordbot/src/discord-mention-routing.ts create mode 100644 services/discordbot/test/discord-mention-routing.test.ts diff --git a/services/discordbot/README.md b/services/discordbot/README.md index 7aa69bb2d4..aaf75ba7a0 100644 --- a/services/discordbot/README.md +++ b/services/discordbot/README.md @@ -13,7 +13,10 @@ iron-control principal; it is never attached to the channel principal. keyed by the new thread (`discord:{guild}:{channel}:{threadId}`). - **`@`-mention inside an existing thread** → the bot answers in that thread. - **Follow-ups inside an authorized thread** append to the same session without a re-mention - only for the original actor, while the root TTL and their current role policy remain valid. + only for the original actor, while the root TTL and their current role policy remain valid. An + unmentioned reply with a canonical `<@user-id>` / `<@!user-id>` mention of another member is + ignored instead of steering Centaur; a direct Centaur mention keeps its normal behavior even if + another member is also named. - **`@centaur stop`** interrupts only the active execution attached to that authorized thread. - **`@centaur approve sha256:…`** atomically consumes one exact, unexpired workflow proposal when the actor's reviewed role is permitted to approve it. It does not create a coding session. diff --git a/services/discordbot/src/discord-ingress.ts b/services/discordbot/src/discord-ingress.ts index 121702042f..31a2acc9a6 100644 --- a/services/discordbot/src/discord-ingress.ts +++ b/services/discordbot/src/discord-ingress.ts @@ -8,6 +8,7 @@ import { resolveDiscordPermissionBundle, type DiscordPermissionBundle, } from "./discord-policy"; +import { discordMentionRoutingDecision } from "./discord-mention-routing"; import type { DiscordbotOptions } from "./types"; const DEFAULT_DELIVERY_TTL_MS = 7 * 24 * 60 * 60 * 1000; @@ -42,6 +43,7 @@ export type DiscordIngressReason = | "bot_message" | "channel_not_allowlisted" | "direct_message" + | "directed_to_other_discord_member" | "duplicate_delivery" | "future_delivery" | "gateway_identity_unverified" @@ -190,6 +192,7 @@ export function discordGatewayEventFromMessage( : {}; const parsed = parseDiscordThreadKey(message.threadId); if (!parsed.guildId || !parsed.channelId) return null; + if (typeof raw.content !== "string") return null; const member = raw.member && typeof raw.member === "object" ? (raw.member as Record) : {}; @@ -203,7 +206,10 @@ export function discordGatewayEventFromMessage( authorIsBot: message.author.isBot === true, authorIsSelf: message.author.isMe === true, channelId: parsed.channelId, - content: message.text, + // Keep the canonical Discord syntax. Chat adapters may strip member + // mentions from their rendered `message.text`, but addressee routing must + // compare immutable user IDs rather than mutable display text. + content: raw.content, createdTimestamp: message.metadata.dateSent.getTime(), gatewayIdentityVerified: true, guildId: parsed.guildId, @@ -295,6 +301,20 @@ async function evaluateAdmission( if (root.policy.fingerprint !== policy.fingerprint) { return deny("policy_changed_requires_root_trigger"); } + // This subscribed-thread path is the legacy all-replies continuation + // mode. Preserve the actor/permission/root checks above, then veto an + // explicit addressee boundary before the message can append to (and steer) + // a Centaur session. Direct Centaur mentions take the mentioned path and + // retain their normal behavior, even when another member is also named. + if ( + discordMentionRoutingDecision( + event.content, + event.isMentioned, + options.applicationId, + ) === "other_member" + ) { + return deny("directed_to_other_discord_member"); + } return accepted(event, root, policy, now); } diff --git a/services/discordbot/src/discord-mention-routing.ts b/services/discordbot/src/discord-mention-routing.ts new file mode 100644 index 0000000000..c4a94fc7d1 --- /dev/null +++ b/services/discordbot/src/discord-mention-routing.ts @@ -0,0 +1,33 @@ +const DISCORD_MEMBER_MENTION_PATTERN = /<@!?(\d{16,22})>/g; + +export type DiscordMentionRoutingDecision = + | "centaur" + | "other_member" + | "unaddressed"; + +/** + * Resolve the addressee boundary from Discord's canonical Gateway content. + * + * `isCentaurMention` is derived by the adapter from Discord's structured + * mention collections. Raw `content` is also inspected because downstream + * adapters may remove mention tokens from their rendered message text. A + * direct Centaur trigger wins when another member is named in the same + * message; otherwise an explicit member mention belongs to that member, not + * to an unmentioned Centaur continuation. + */ +export function discordMentionRoutingDecision( + content: string, + isCentaurMention: boolean, + centaurUserId: string, +): DiscordMentionRoutingDecision { + const mentionedUserIds = new Set( + [...content.matchAll(DISCORD_MEMBER_MENTION_PATTERN)] + .map((match) => match[1]) + .filter((userId): userId is string => userId !== undefined), + ); + + if (isCentaurMention || mentionedUserIds.has(centaurUserId)) { + return "centaur"; + } + return mentionedUserIds.size > 0 ? "other_member" : "unaddressed"; +} diff --git a/services/discordbot/test/chat-sdk-emulate.test.ts b/services/discordbot/test/chat-sdk-emulate.test.ts index 1833dd5a3c..ad0d2d0fea 100644 --- a/services/discordbot/test/chat-sdk-emulate.test.ts +++ b/services/discordbot/test/chat-sdk-emulate.test.ts @@ -48,6 +48,7 @@ import { admitDiscordGatewayMessage } from "../src/discord-ingress"; const BOT_TOKEN = "discordbot-emulate-token"; const APP_ID = "900000000000000001"; const USER_ID = "100000000000000001"; +const OTHER_USER_ID = "100000000000000002"; const TRIGGER_BOT_ID = "400000000000000001"; const GUILD_ID = "200000000000000001"; const CHANNEL_ID = "300000000000000001"; @@ -240,6 +241,53 @@ describe("discordbot", () => { ); }); + it("does not route an unmentioned reply addressed to another member but honors a direct bot mention", async () => { + const threadId = discordApi.nextId(); + discordApi.seedThreadChannel(threadId, CHANNEL_ID); + + const rootMentionId = await dispatchMessage({ + channelId: threadId, + content: `<@${APP_ID}> establish the routing test thread`, + mention: true, + thread: { id: threadId, parentId: CHANNEL_ID }, + }); + await waitForSettle(threadId, rootMentionId); + const appendCount = codexApi.appends.length; + const executeCount = codexApi.executes.length; + + const addressedElsewhere = + `<@${OTHER_USER_ID}> great. Can you write a small summary of where the service is at?`; + await dispatchMessage({ + channelId: threadId, + content: addressedElsewhere, + mentionedUserIds: [OTHER_USER_ID], + thread: { id: threadId, parentId: CHANNEL_ID }, + }); + await sleep(50); + expect(codexApi.appends).toHaveLength(appendCount); + expect(codexApi.executes).toHaveLength(executeCount); + + const directMentionId = await dispatchMessage({ + channelId: threadId, + content: + `<@${APP_ID}> ask <@${OTHER_USER_ID}> for context, then write the summary`, + mention: true, + mentionedUserIds: [APP_ID, OTHER_USER_ID], + thread: { id: threadId, parentId: CHANNEL_ID }, + }); + await waitForSettle(threadId, directMentionId); + expect(codexApi.appends).toHaveLength(appendCount + 1); + expect(codexApi.executes).toHaveLength(executeCount + 1); + expect(codexApi.executes.at(-1)?.body.idempotency_key).toBe( + directMentionId, + ); + expect( + codexApi.appends.flatMap((append) => + sessionMessageTexts(append.body.messages), + ), + ).not.toContain(addressedElsewhere); + }); + it("creates, names, and answers in a bot-created thread for a channel mention", async () => { const mentionId = await dispatchMessage({ channelId: CHANNEL_ID, @@ -1454,6 +1502,7 @@ async function dispatchMessage(input: { content: string; guildId?: string; mention?: boolean; + mentionedUserIds?: string[]; /** Existing-thread tests seed the durable root production creates upstream. */ preauthorizeRoot?: boolean; roleIds?: string[]; @@ -1499,7 +1548,9 @@ async function dispatchMessage(input: { guild_id: input.guildId ?? GUILD_ID, member: { roles: input.roleIds ?? [TRIGGER_ROLE_ID] }, mention_roles: [], - mentions: input.mention ? [{ id: APP_ID }] : [], + mentions: ( + input.mentionedUserIds ?? (input.mention ? [APP_ID] : []) + ).map((id) => ({ id })), ...(input.thread ? { thread: { id: input.thread.id, parent_id: input.thread.parentId } } : {}), diff --git a/services/discordbot/test/discord-ingress.test.ts b/services/discordbot/test/discord-ingress.test.ts index ea1937e8db..c9c2af4716 100644 --- a/services/discordbot/test/discord-ingress.test.ts +++ b/services/discordbot/test/discord-ingress.test.ts @@ -261,6 +261,59 @@ describe("Discord Gateway admission", () => { } }); + it("vetoes another member as an unmentioned addressee but keeps direct Centaur mentions", async () => { + const { audits, logger, state } = await harness(); + const configured = options(); + await admitDiscordGatewayMessage( + event(THREAD), + configured, + state, + logger, + NOW, + ); + + const addressedElsewhere = event("600000000000000065", { + content: + `<@${OTHER_USER}> great. Can you write a small summary of where the service is at?`, + isMentioned: false, + threadId: THREAD, + }); + expect( + await admitDiscordGatewayMessage( + addressedElsewhere, + configured, + state, + logger, + NOW + 100, + ), + ).toBeNull(); + expect(audits.at(-1)?.data.reason).toBe( + "directed_to_other_discord_member", + ); + + const directCentaurMention = event("600000000000000066", { + content: + `<@${APP}> ask <@${OTHER_USER}> for context, then write the summary`, + isMentioned: true, + threadId: THREAD, + }); + expect( + await admitDiscordGatewayMessage( + directCentaurMention, + configured, + state, + logger, + NOW + 100, + ), + ).toEqual( + expect.objectContaining({ + actorId: USER, + decision: "allow", + messageId: directCentaurMention.messageId, + }), + ); + }); + it("accepts only an actor-scoped, idempotent stop control", async () => { const { audits, logger, state } = await harness(); const configured = options(); diff --git a/services/discordbot/test/discord-mention-routing.test.ts b/services/discordbot/test/discord-mention-routing.test.ts new file mode 100644 index 0000000000..a9a74b725d --- /dev/null +++ b/services/discordbot/test/discord-mention-routing.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "bun:test"; +import { discordMentionRoutingDecision } from "../src/discord-mention-routing"; + +const CENTAUR_ID = "900000000000000001"; +const OTHER_USER_ID = "100000000000000002"; + +describe("Discord mention routing", () => { + it("treats canonical other-member mentions as an addressee boundary", () => { + const actionable = + `<@${OTHER_USER_ID}> great. Can you write a small summary of where the service is at?`; + + expect( + discordMentionRoutingDecision(actionable, false, CENTAUR_ID), + ).toBe("other_member"); + expect( + discordMentionRoutingDecision( + `Can you send the summary to <@!${OTHER_USER_ID}>?`, + false, + CENTAUR_ID, + ), + ).toBe("other_member"); + expect( + discordMentionRoutingDecision( + "@someone great. Can you write a small summary?", + false, + CENTAUR_ID, + ), + ).toBe("unaddressed"); + }); + + it("keeps a direct Centaur mention authoritative when another member is named", () => { + const content = + `<@${CENTAUR_ID}> ask <@${OTHER_USER_ID}> for context, then write the summary`; + + expect(discordMentionRoutingDecision(content, true, CENTAUR_ID)).toBe( + "centaur", + ); + // Canonical raw content also preserves the direct trigger if an adapter's + // rendered mention flag is unavailable downstream. + expect(discordMentionRoutingDecision(content, false, CENTAUR_ID)).toBe( + "centaur", + ); + }); +});