mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
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 <goffern@users.noreply.github.com>
---------
Co-authored-by: goffern <goffern@users.noreply.github.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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", () => {
|
||||
|
||||
@@ -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 `(?<![a-z0-9_])@${escapeRegExp(username)}(?![a-z0-9_]|[.:-]+[a-z0-9_])`;
|
||||
}
|
||||
|
||||
export function matchesMattermostBotMention(
|
||||
text: string,
|
||||
botUsername: string | undefined,
|
||||
): boolean {
|
||||
if (!botUsername) {
|
||||
return false;
|
||||
}
|
||||
return new RegExp(buildMattermostBotMentionPattern(botUsername), "i").test(text);
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip bot mention from message text while preserving newlines and
|
||||
* block-level Markdown formatting (headings, lists, blockquotes).
|
||||
@@ -28,20 +45,24 @@ export function normalizeMention(text: string, mention: string | undefined): str
|
||||
if (!mention) {
|
||||
return text.trim();
|
||||
}
|
||||
const escaped = mention.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||
const hasMentionRe = new RegExp(`@${escaped}\\b`, "i");
|
||||
const leadingMentionRe = new RegExp(`^([\\t ]*)@${escaped}\\b[\\t ]*`, "i");
|
||||
const trailingMentionRe = new RegExp(`[\\t ]*@${escaped}\\b[\\t ]*$`, "i");
|
||||
const pattern = buildMattermostBotMentionPattern(mention);
|
||||
const hasMentionRe = new RegExp(pattern, "i");
|
||||
const leadingMentionRe = new RegExp(`^([\\t ]*)${pattern}[\\t ]*`, "i");
|
||||
const trailingMentionRe = new RegExp(`[\\t ]*${pattern}[\\t ]*$`, "i");
|
||||
const normalizedLines = text.split("\n").map((line) => {
|
||||
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() === "",
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -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) : [];
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user