From fe7a85960d64ea9923a422930a983385d5d5cc0d Mon Sep 17 00:00:00 2001 From: goffern <10464170+goffern@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:31:50 +0800 Subject: [PATCH] fix(mattermost): anchor bot mention matching on username boundaries (#129555) * fix(mattermost): anchor bot mention matching on username boundaries The wake check used a bare substring test and the mention stripper used \b word boundaries, but Mattermost usernames match ^[a-z0-9.\-_]+$ (server model/user.go), where "." and "-" are username characters. A mention of a different user whose handle starts with the bot name ("@clawdia", "@claw.dia", "@claw-ops") or an embedded handle ("bob@claw") woke the bot and answered a message addressed to someone else, and stripping ate the bot-name prefix out of the other user's handle, delivering a mangled body to the model. Anchor both sides of the mention pattern on the username character class, and stop rewriting lines that contain no mention at all: the whitespace collapse ran on every line, destroying code-block and table alignment in messages that merely accompanied a mention. Detection and stripping share one anchored pattern helper; the wake decision now uses it instead of the lowercase substring check. * fix(mattermost): align bot mentions with server punctuation rules Share one upstream-accurate mention pattern across activation and normalization while preserving local, federated, and punctuation-adjacent behavior at the real posted-event boundary. Co-authored-by: goffern --------- Co-authored-by: goffern Co-authored-by: Peter Steinberger --- .../monitor-helpers.test-support.ts | 81 ++++++++++++++++++- .../src/mattermost/monitor-helpers.ts | 35 ++++++-- .../src/mattermost/monitor-posts.ts | 8 +- .../monitor.inbound-system-event.test.ts | 67 +++++++++++++++ 4 files changed, 177 insertions(+), 14 deletions(-) diff --git a/extensions/mattermost/src/mattermost/monitor-helpers.test-support.ts b/extensions/mattermost/src/mattermost/monitor-helpers.test-support.ts index 580324f2ccb9..cb144ad1521d 100644 --- a/extensions/mattermost/src/mattermost/monitor-helpers.test-support.ts +++ b/extensions/mattermost/src/mattermost/monitor-helpers.test-support.ts @@ -1,6 +1,49 @@ // Mattermost test support covers monitor helpers plugin behavior. import { describe, expect, it } from "vitest"; -import { normalizeMention, shouldDropEmptyMattermostBody } from "./monitor-helpers.js"; +import { + matchesMattermostBotMention, + normalizeMention, + shouldDropEmptyMattermostBody, +} from "./monitor-helpers.js"; + +describe("matchesMattermostBotMention", () => { + it.each([ + "@echobot hello", + "hey @echobot check this", + "(@echobot)", + "hello.@echobot", + "hello-@echobot", + "hello:@echobot", + "@echobot.", + "@echobot...", + "@echobot-", + "thanks @echobot: run it", + "thanks @echobot:", + "team:@echobot hello", + "@EchoBot hello", + "@echobot", + ])("matches a real bot mention: %j", (text) => { + expect(matchesMattermostBotMention(text, "echobot")).toBe(true); + }); + + // Mattermost usernames allow [a-z0-9._-]; these are mentions of other users. + it.each([ + "@echobotdia hello", + "@echobot.dia hello", + "@echobot-ops please review", + "@echobot_2 ping", + "@echobot:remote hello", + "@echobot:remote.example hello", + "@echobot::remote hello", + "mail me at bob@echobot later", + ])("does not match a longer username or embedded handle: %j", (text) => { + expect(matchesMattermostBotMention(text, "echobot")).toBe(false); + }); + + it("returns false without a bot username", () => { + expect(matchesMattermostBotMention("@echobot hello", undefined)).toBe(false); + }); +}); describe("normalizeMention", () => { it("returns trimmed text when no mention provided", () => { @@ -80,6 +123,42 @@ describe("normalizeMention", () => { const result = normalizeMention(input, "echobot"); expect(result).toBe(" code line 1\n code line 2"); }); + + it.each([ + "@echobot.dia hello", + "@echobot-ops please review", + "@echobotdia hello", + "@echobot:remote hello", + "@echobot:remote.example hello", + "mail me at bob@echobot later", + ])("leaves other users' handles intact: %j", (input) => { + expect(normalizeMention(input, "echobot")).toBe(input); + }); + + it.each([ + { input: "hello.@echobot", expected: "hello." }, + { input: "hello-@echobot", expected: "hello-" }, + { input: "hello:@echobot", expected: "hello:" }, + { input: "@echobot.", expected: "." }, + { input: "@echobot-", expected: "-" }, + { input: "@echobot: hello", expected: ": hello" }, + ])("preserves punctuation around a real mention: $input", ({ input, expected }) => { + expect(normalizeMention(input, "echobot")).toBe(expected); + }); + + it("preserves table padding on lines without the mention", () => { + const input = "@echobot see table\n| a | b |\n| aaa | bbb |"; + expect(normalizeMention(input, "echobot")).toBe("see table\n| a | b |\n| aaa | bbb |"); + }); + + it("preserves code-fence alignment on lines without the mention", () => { + const input = "@echobot look\n```\nx = 1 # aligned\n```"; + expect(normalizeMention(input, "echobot")).toBe("look\n```\nx = 1 # aligned\n```"); + }); + + it("still collapses doubled spaces on the line the mention was removed from", () => { + expect(normalizeMention("hey @echobot check", "echobot")).toBe("hey check"); + }); }); describe("shouldDropEmptyMattermostBody", () => { diff --git a/extensions/mattermost/src/mattermost/monitor-helpers.ts b/extensions/mattermost/src/mattermost/monitor-helpers.ts index 2892a58939ca..62b116eaac87 100644 --- a/extensions/mattermost/src/mattermost/monitor-helpers.ts +++ b/extensions/mattermost/src/mattermost/monitor-helpers.ts @@ -2,6 +2,7 @@ import { formatInboundFromLabel as formatInboundFromLabelShared } from "openclaw/plugin-sdk/channel-inbound"; import { resolveThreadSessionKeys as resolveThreadSessionKeysShared } from "openclaw/plugin-sdk/routing"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { escapeRegExp } from "openclaw/plugin-sdk/text-utility-runtime"; import { rawDataToString } from "openclaw/plugin-sdk/webhook-ingress"; export { rawDataToString }; @@ -20,6 +21,22 @@ export function resolveThreadSessionKeys(params: { }); } +// Server mentions allow surrounding sentence punctuation, while punctuation +// followed by username characters belongs to another local or remote account. +function buildMattermostBotMentionPattern(username: string): string { + return `(? { - const hadMention = hasMentionRe.test(line); + // Lines without the mention keep their exact bytes: the whitespace collapse + // below would otherwise destroy code-block and table alignment repo-wide. + if (!hasMentionRe.test(line)) { + return { text: line, mentionOnlyBlank: false }; + } const normalizedLine = line .replace(leadingMentionRe, "$1") .replace(trailingMentionRe, "") - .replace(new RegExp(`@${escaped}\\b`, "gi"), "") + .replace(new RegExp(pattern, "gi"), "") .replace(/(\S)[ \t]{2,}/g, "$1 "); return { text: normalizedLine, - mentionOnlyBlank: hadMention && normalizedLine.trim() === "", + mentionOnlyBlank: normalizedLine.trim() === "", }; }); diff --git a/extensions/mattermost/src/mattermost/monitor-posts.ts b/extensions/mattermost/src/mattermost/monitor-posts.ts index ed22c39ceb6f..47adfa93c6af 100644 --- a/extensions/mattermost/src/mattermost/monitor-posts.ts +++ b/extensions/mattermost/src/mattermost/monitor-posts.ts @@ -10,7 +10,6 @@ import { } from "openclaw/plugin-sdk/context-visibility-runtime"; import { resolvePinnedMainDmOwnerFromAllowlist } from "openclaw/plugin-sdk/security-runtime"; import { - normalizeLowercaseStringOrEmpty, normalizeOptionalString, normalizeTrimmedStringList, uniqueStrings, @@ -26,6 +25,7 @@ import { resolveMattermostPendingHistoryKey } from "./monitor-context.js"; import { buildMattermostEventPlan } from "./monitor-event-plan.js"; import { formatInboundFromLabel, + matchesMattermostBotMention, normalizeMention, shouldDropEmptyMattermostBody, } from "./monitor-helpers.js"; @@ -247,11 +247,7 @@ export function createMattermostPostHandler(monitor: MattermostMonitorContext) { const mentionRegexes = core.channel.mentions.buildMentionRegexes(cfg, route.agentId); const wasMentioned = kind !== "direct" && - ((botUsername - ? normalizeLowercaseStringOrEmpty(rawText).includes( - `@${normalizeLowercaseStringOrEmpty(botUsername)}`, - ) - : false) || + (matchesMattermostBotMention(rawText, botUsername) || core.channel.mentions.matchesMentionPatterns(rawText, mentionRegexes)); const oncharEnabled = account.chatmode === "onchar" && kind !== "direct"; const oncharPrefixes = oncharEnabled ? resolveOncharPrefixes(account.oncharPrefixes) : []; diff --git a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts index 1e3d991c0fd8..77bdd4e9eb76 100644 --- a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts +++ b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts @@ -1338,6 +1338,73 @@ describe("mattermost inbound user posts", () => { expect(ctx?.Provider).toBe("mattermost"); }); + it.each([ + { message: "@openclawdia hello", expectedBody: null }, + { message: "@openclaw:remote.example hello", expectedBody: null }, + { message: "hello.@openclaw", expectedBody: "hello." }, + { message: "hello-@openclaw", expectedBody: "hello-" }, + { message: "hello:@openclaw", expectedBody: "hello:" }, + { message: "@openclaw.", expectedBody: "." }, + { message: "@openclaw-", expectedBody: "-" }, + { message: "@openclaw: hello", expectedBody: ": hello" }, + ])( + "dispatches only genuine mention-required posts: $message", + async ({ message, expectedBody }) => { + const socket = new FakeWebSocket(); + const abortController = new AbortController(); + mockState.abortController = abortController; + const verboseDebug = vi.fn(); + const config: OpenClawConfig = { + channels: { + mattermost: { + enabled: true, + baseUrl: "https://mattermost.example.com", + botToken: "bot-token", + // No chatmode: "onmessage" would force requireMention off (accounts.ts). + dmPolicy: "open", + groupPolicy: "open", + requireMention: true, + }, + }, + }; + mockState.runtimeCore = createRuntimeCore(config, undefined, { verboseDebug }); + + const monitor = monitorMattermostProvider({ + config, + runtime: testRuntime(), + abortSignal: abortController.signal, + webSocketFactory: () => socket, + }); + + await vi.waitFor(() => { + expect(socket.openListenerCount).toBeGreaterThan(0); + }); + socket.emitOpen(); + + await emitMattermostChannelPost(socket, { + id: "post-mention-boundary", + message, + }); + + if (expectedBody === null) { + await vi.waitFor(() => { + expect(verboseDebug).toHaveBeenCalledWith( + expect.stringContaining("drop group message (missing mention"), + ); + }); + expect(mockState.dispatchInboundMessage).not.toHaveBeenCalled(); + } else { + expect(mockState.dispatchInboundMessage).toHaveBeenCalledTimes(1); + expect(mockState.dispatchInboundMessage.mock.calls.at(0)?.[0].ctx.BodyForAgent).toBe( + expectedBody, + ); + } + abortController.abort(); + socket.emitClose(1000); + await monitor; + }, + ); + it("merges Mattermost progress preview updates and clears after message-tool delivery", async () => { const socket = new FakeWebSocket(); const abortController = new AbortController();