From 8a98c08c8a18919855c5f1be6d092127eade39a3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Apr 2026 09:36:36 +0100 Subject: [PATCH] fix(mattermost): avoid system events for user posts --- CHANGELOG.md | 1 + .../monitor.inbound-system-event.test.ts | 337 ++++++++++++++++++ .../mattermost/src/mattermost/monitor.ts | 10 - 3 files changed, 338 insertions(+), 10 deletions(-) create mode 100644 extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eea22a5a0d..08d2efdd7d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai - Auto-reply/commands: stop bare `/reset` and `/new` after reset hooks acknowledge the command, so non-ACP channels no longer fall through into empty provider calls while `/reset ` and `/new ` still seed the next model turn. Fixes #73367. Thanks @hoyanhan and @wenxu007. - Auto-reply: preserve voice-note media from silent turns while continuing to suppress text and non-voice media, so `NO_REPLY` TTS replies still deliver the requested audio bubble. (#73406) Thanks @zqchris. +- Channels/Mattermost: stop enqueueing regular inbound posts as system events, so Mattermost user messages reach the model only as user-role inbound-envelope content instead of also appearing as `System: Mattermost message...` directives. Fixes #71795. Thanks @juan-flores077. - Agents/Anthropic: send implicit Anthropic beta headers only to direct public Anthropic endpoints, including OAuth, so custom Anthropic-compatible providers no longer mis-handle unsupported beta flags unless explicitly configured. Refs #73346. Thanks @byBrodowski. - Skills: require explicit `skills.entries.coding-agent.enabled` before exposing the bundled coding-agent skill, so installs with Codex on PATH but no OpenAI auth do not silently offer Codex delegation. Fixes #73358. Thanks @LaFleurAdvertising and @Sanjays2402. - Plugins/startup: precompute bundled runtime mirror fingerprints before taking the mirror lock and keep Docker bundled plugin runtime deps/mirrors in a Docker-managed volume instead of the Windows/WSL config bind mount, so cold starts avoid slow host-volume mirror writes. Fixes #73339. Thanks @1yihui. diff --git a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts new file mode 100644 index 00000000000..7fa1ccfc310 --- /dev/null +++ b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts @@ -0,0 +1,337 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig, RuntimeEnv } from "./runtime-api.js"; + +class FakeWebSocket { + public readonly sent: string[] = []; + private readonly openListeners: Array<() => void> = []; + private readonly messageListeners: Array<(data: Buffer) => void | Promise> = []; + private readonly closeListeners: Array<(code: number, reason: Buffer) => void> = []; + private readonly errorListeners: Array<(err: unknown) => void> = []; + + on(event: "open", listener: () => void): void; + on(event: "message", listener: (data: Buffer) => void | Promise): void; + on(event: "close", listener: (code: number, reason: Buffer) => void): void; + on(event: "error", listener: (err: unknown) => void): void; + on(event: "open" | "message" | "close" | "error", listener: unknown): void { + if (event === "open") { + this.openListeners.push(listener as () => void); + return; + } + if (event === "message") { + this.messageListeners.push(listener as (data: Buffer) => void | Promise); + return; + } + if (event === "close") { + this.closeListeners.push(listener as (code: number, reason: Buffer) => void); + return; + } + this.errorListeners.push(listener as (err: unknown) => void); + } + + send(data: string): void { + this.sent.push(data); + } + + close(): void {} + + terminate(): void {} + + get openListenerCount(): number { + return this.openListeners.length; + } + + emitOpen(): void { + for (const listener of this.openListeners) { + listener(); + } + } + + async emitMessage(payload: unknown): Promise { + const buffer = Buffer.from(JSON.stringify(payload), "utf8"); + await Promise.all(this.messageListeners.map((listener) => Promise.resolve(listener(buffer)))); + } + + emitClose(code: number, reason = ""): void { + const buffer = Buffer.from(reason, "utf8"); + for (const listener of this.closeListeners) { + listener(code, buffer); + } + } + + emitError(err: unknown): void { + for (const listener of this.errorListeners) { + listener(err); + } + } +} + +const mockState = vi.hoisted(() => ({ + abortController: undefined as AbortController | undefined, + createMattermostClient: vi.fn(), + createMattermostDraftStream: vi.fn(), + dispatchReplyFromConfig: vi.fn(), + enqueueSystemEvent: vi.fn(), + fetchMattermostMe: vi.fn(), + registerMattermostMonitorSlashCommands: vi.fn(), + registerPluginHttpRoute: vi.fn(), + resolveChannelInfo: vi.fn(), + resolveMattermostMedia: vi.fn(), + resolveUserInfo: vi.fn(), + runtimeCore: undefined as unknown, + updateMattermostPost: vi.fn(), +})); + +vi.mock("./client.js", async () => { + const actual = await vi.importActual("./client.js"); + return { + ...actual, + createMattermostClient: mockState.createMattermostClient, + fetchMattermostMe: mockState.fetchMattermostMe, + normalizeMattermostBaseUrl: (value: string | undefined) => value?.trim() ?? "", + updateMattermostPost: mockState.updateMattermostPost, + }; +}); + +vi.mock("./draft-stream.js", () => ({ + buildMattermostToolStatusText: () => "Working", + createMattermostDraftStream: mockState.createMattermostDraftStream, +})); + +vi.mock("./monitor-resources.js", () => ({ + createMattermostMonitorResources: () => ({ + resolveMattermostMedia: mockState.resolveMattermostMedia, + sendTypingIndicator: vi.fn(async () => {}), + resolveChannelInfo: mockState.resolveChannelInfo, + resolveUserInfo: mockState.resolveUserInfo, + updateModelPickerPost: vi.fn(async () => {}), + }), +})); + +vi.mock("./monitor-slash.js", () => ({ + registerMattermostMonitorSlashCommands: mockState.registerMattermostMonitorSlashCommands, +})); + +vi.mock("./runtime-api.js", async () => { + const actual = await vi.importActual("./runtime-api.js"); + return { + ...actual, + buildAgentMediaPayload: vi.fn(() => ({})), + createChannelPairingController: vi.fn(() => ({ + readStoreForDmPolicy: vi.fn(async () => []), + upsertPairingRequest: vi.fn(async () => ({ code: "123456", created: true })), + })), + createChannelReplyPipeline: vi.fn(() => ({ + onModelSelected: vi.fn(), + typingCallbacks: {}, + })), + readStoreAllowFromForDmPolicy: vi.fn(async () => []), + registerPluginHttpRoute: mockState.registerPluginHttpRoute, + resolveChannelMediaMaxBytes: vi.fn(() => 8 * 1024 * 1024), + warnMissingProviderGroupPolicyFallbackOnce: vi.fn(), + }; +}); + +function createRuntimeCore(cfg: OpenClawConfig) { + return { + config: { + current: () => cfg, + }, + logging: { + shouldLogVerbose: () => false, + getChildLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), + }, + media: { + mediaKindFromMime: () => "document", + }, + system: { + enqueueSystemEvent: mockState.enqueueSystemEvent, + }, + channel: { + activity: { + record: vi.fn(), + }, + commands: { + shouldHandleTextCommands: () => false, + }, + debounce: { + resolveInboundDebounceMs: () => 0, + createInboundDebouncer: (params: { + onFlush: (entries: T[]) => Promise | void; + }) => ({ + enqueue: async (entry: T) => { + await params.onFlush([entry]); + }, + }), + }, + groups: { + resolveRequireMention: () => false, + }, + media: { + fetchRemoteMedia: vi.fn(), + saveMediaBuffer: vi.fn(), + }, + mentions: { + buildMentionRegexes: () => [], + matchesMentionPatterns: () => false, + }, + pairing: { + buildPairingReply: () => "pairing required", + }, + reply: { + createReplyDispatcherWithTyping: vi.fn(() => ({ + dispatcher: {}, + replyOptions: {}, + markDispatchIdle: vi.fn(), + markRunComplete: vi.fn(), + })), + dispatchReplyFromConfig: mockState.dispatchReplyFromConfig, + finalizeInboundContext: (context: unknown) => context, + formatInboundEnvelope: (params: { channel: string; from: string; body: string }) => + `${params.channel} ${params.from}\n${params.body}`, + resolveHumanDelayConfig: () => ({}), + withReplyDispatcher: async (params: { run: () => unknown; onSettled?: () => void }) => { + try { + return await params.run(); + } finally { + params.onSettled?.(); + } + }, + }, + routing: { + resolveAgentRoute: () => ({ + accountId: "default", + agentId: "main", + mainSessionKey: "mattermost:default:channel:chan-1", + sessionKey: "mattermost:default:channel:chan-1", + }), + }, + session: { + resolveStorePath: () => "/tmp/openclaw-test-sessions.json", + updateLastRoute: vi.fn(async () => {}), + }, + text: { + chunkMarkdownTextWithMode: (text: string) => [text], + convertMarkdownTables: (text: string) => text, + hasControlCommand: () => false, + resolveChunkMode: () => "off", + resolveMarkdownTableMode: () => "off", + resolveTextChunkLimit: () => 4000, + }, + }, + }; +} + +const testConfig: OpenClawConfig = { + channels: { + mattermost: { + enabled: true, + baseUrl: "https://mattermost.example.com", + botToken: "bot-token", + chatmode: "onmessage", + dmPolicy: "open", + groupPolicy: "open", + }, + }, +}; + +vi.mock("../runtime.js", () => ({ + getMattermostRuntime: () => mockState.runtimeCore, +})); + +const testRuntime = (): RuntimeEnv => + ({ + log: vi.fn(), + error: vi.fn(), + exit: ((code: number): never => { + throw new Error(`exit ${code}`); + }) as RuntimeEnv["exit"], + }) satisfies RuntimeEnv; + +describe("mattermost inbound user posts", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockState.abortController = undefined; + mockState.runtimeCore = createRuntimeCore(testConfig); + mockState.createMattermostClient.mockReturnValue({}); + mockState.createMattermostDraftStream.mockReturnValue({ + update: vi.fn(), + stop: vi.fn(async () => {}), + }); + mockState.fetchMattermostMe.mockResolvedValue({ + id: "bot-user", + username: "openclaw", + update_at: 1, + }); + mockState.registerMattermostMonitorSlashCommands.mockResolvedValue(undefined); + mockState.registerPluginHttpRoute.mockReturnValue(vi.fn()); + mockState.resolveChannelInfo.mockResolvedValue({ + id: "chan-1", + name: "town-square", + display_name: "Town Square", + team_id: "team-1", + type: "O", + }); + mockState.resolveMattermostMedia.mockResolvedValue([]); + mockState.resolveUserInfo.mockResolvedValue({ id: "user-1", username: "alice" }); + mockState.dispatchReplyFromConfig.mockImplementation(async () => { + mockState.abortController?.abort(); + }); + }); + + it("does not enqueue regular user posts as system events", async () => { + const socket = new FakeWebSocket(); + const abortController = new AbortController(); + mockState.abortController = abortController; + const { monitorMattermostProvider } = await import("./monitor.js"); + + const monitor = monitorMattermostProvider({ + config: testConfig, + runtime: testRuntime(), + abortSignal: abortController.signal, + webSocketFactory: () => socket, + }); + + await vi.waitFor(() => { + expect(socket.openListenerCount).toBeGreaterThan(0); + }); + socket.emitOpen(); + + await socket.emitMessage({ + event: "posted", + data: { + channel_id: "chan-1", + channel_name: "town-square", + channel_display_name: "Town Square", + sender_name: "alice", + post: JSON.stringify({ + id: "post-1", + channel_id: "chan-1", + user_id: "user-1", + message: "hello from mattermost", + create_at: 1_714_000_000_000, + }), + }, + broadcast: { + channel_id: "chan-1", + user_id: "user-1", + }, + }); + socket.emitClose(1000); + await monitor; + + expect(mockState.enqueueSystemEvent).not.toHaveBeenCalled(); + expect(mockState.dispatchReplyFromConfig).toHaveBeenCalledTimes(1); + expect(mockState.dispatchReplyFromConfig.mock.calls[0]?.[0].ctx).toMatchObject({ + BodyForAgent: "hello from mattermost", + ConversationLabel: "Town Square id:chan-1", + MessageSid: "post-1", + OriginatingChannel: "mattermost", + Provider: "mattermost", + }); + }); +}); diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 005676b0a84..739e0aadecd 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -1488,16 +1488,6 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} directId: senderId, }); - const preview = bodyText.replace(/\s+/g, " ").slice(0, 160); - const inboundLabel = - kind === "direct" - ? `Mattermost DM from ${senderName}` - : `Mattermost message in ${roomLabel} from ${senderName}`; - core.system.enqueueSystemEvent(`${inboundLabel}: ${preview}`, { - sessionKey, - contextKey: `mattermost:message:${channelId}:${post.id ?? "unknown"}`, - }); - const textWithId = `${bodyText}\n[mattermost message id: ${post.id ?? "unknown"} channel: ${channelId}]`; const body = core.channel.reply.formatInboundEnvelope({ channel: "Mattermost",