diff --git a/CHANGELOG.md b/CHANGELOG.md index 329a1e659fc..1d79fc7cf2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Docs: https://docs.openclaw.ai - Plugins/contracts: resolve runtime manifest-contract plugin owners from one plugin index plus manifest pass instead of rebuilding manifest metadata separately for all owners and enabled owners. Thanks @shakkernerd. - Plugins/extractors: reuse one manifest registry pass while resolving bundled document and web-content extractor plugins instead of rereading manifests for compatibility and enablement filtering. Thanks @shakkernerd. - Plugins/registry: resolve lookup-table owner maps for providers, CLI backends, setup providers, command aliases, model catalogs, channel configs, and manifest contracts while preserving setup-only CLI backend ownership. Thanks @shakkernerd. +- Mattermost: keep direct-message replies top-level by suppressing reply roots for DM delivery while preserving channel and group thread roots, and derive inbound chat kind from the trusted channel lookup instead of the websocket event channel type. Carries forward #60115, #55186, #72305, and #72659; refs #59758, #59981, #59791, and #57565. Thanks @vincentkoc, @jwchmodx, and @hnykda. - Process/Windows: decode command stdout and stderr from raw bytes with console-codepage awareness, while preserving valid UTF-8 output and multibyte characters split across chunks. Fixes #50519. Thanks @iready, @kevinten10, @zhangyongjie1997, @knightplat-blip, @heiqishi666, and @slepybear. - Bonjour/Windows: hide the bundled mDNS advertiser's Windows ARP shell probe so Gateway startup no longer flashes command-prompt windows. Fixes #70238. Thanks @alexandre-leng, @PratikRai0101, @infinitypacific, and @tomerpeled. - Agents/bootstrap: dedupe hook-injected bootstrap context files by workspace-relative path and store normalized resolved paths so duplicate relative and absolute hook paths no longer depend on the process cwd. (#59344; fixes #59319; related #56721, #56725, and #57587) Thanks @koen666. diff --git a/extensions/mattermost/src/mattermost/monitor-gating.test.ts b/extensions/mattermost/src/mattermost/monitor-gating.test.ts index e818467c626..e82bb3b2355 100644 --- a/extensions/mattermost/src/mattermost/monitor-gating.test.ts +++ b/extensions/mattermost/src/mattermost/monitor-gating.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { evaluateMattermostMentionGate, mapMattermostChannelTypeToChatType, + resolveMattermostTrustedChatKind, } from "./monitor-gating.js"; describe("mattermost monitor gating", () => { @@ -13,6 +14,23 @@ describe("mattermost monitor gating", () => { expect(mapMattermostChannelTypeToChatType(undefined)).toBe("channel"); }); + it("derives chat kind from trusted channel lookup before fallback state", () => { + expect( + resolveMattermostTrustedChatKind({ + channelType: "O", + fallback: "direct", + }), + ).toBe("channel"); + expect( + resolveMattermostTrustedChatKind({ + channelType: "D", + fallback: "channel", + }), + ).toBe("direct"); + expect(resolveMattermostTrustedChatKind({ fallback: "group" })).toBe("group"); + expect(resolveMattermostTrustedChatKind({})).toBe("channel"); + }); + it("drops non-mentioned traffic when onchar is enabled but not triggered", () => { const resolveRequireMention = vi.fn(() => true); diff --git a/extensions/mattermost/src/mattermost/monitor-gating.ts b/extensions/mattermost/src/mattermost/monitor-gating.ts index cb0bf7e3913..20f60807756 100644 --- a/extensions/mattermost/src/mattermost/monitor-gating.ts +++ b/extensions/mattermost/src/mattermost/monitor-gating.ts @@ -14,6 +14,17 @@ export function mapMattermostChannelTypeToChatType(channelType?: string | null): return "channel"; } +export function resolveMattermostTrustedChatKind(params: { + channelType?: string | null; + fallback?: ChatType; +}): ChatType { + const channelType = params.channelType?.trim(); + if (channelType) { + return mapMattermostChannelTypeToChatType(channelType); + } + return params.fallback ?? "channel"; +} + export type MattermostRequireMentionResolverInput = { cfg: OpenClawConfig; channel: "mattermost"; diff --git a/extensions/mattermost/src/mattermost/monitor.test.ts b/extensions/mattermost/src/mattermost/monitor.test.ts index 0529376b7ec..8509540c01c 100644 --- a/extensions/mattermost/src/mattermost/monitor.test.ts +++ b/extensions/mattermost/src/mattermost/monitor.test.ts @@ -162,6 +162,7 @@ describe("resolveMattermostReplyRootId with block streaming payloads", () => { // mode, the deliver callback should still use the existing threadRootId. expect( resolveMattermostReplyRootId({ + kind: "channel", threadRootId: "thread-root-1", replyToId: "streamed-reply-id", }), @@ -173,6 +174,7 @@ describe("resolveMattermostReplyRootId with block streaming payloads", () => { // inbound post id as replyToId from the "all" threading mode. expect( resolveMattermostReplyRootId({ + kind: "channel", replyToId: "inbound-post-for-threading", }), ).toBe("inbound-post-for-threading"); @@ -183,6 +185,7 @@ describe("resolveMattermostReplyRootId", () => { it("uses replyToId for top-level replies", () => { expect( resolveMattermostReplyRootId({ + kind: "channel", replyToId: "inbound-post-123", }), ).toBe("inbound-post-123"); @@ -191,6 +194,7 @@ describe("resolveMattermostReplyRootId", () => { it("keeps the thread root when replying inside an existing thread", () => { expect( resolveMattermostReplyRootId({ + kind: "channel", threadRootId: "thread-root-456", replyToId: "child-post-789", }), @@ -198,7 +202,36 @@ describe("resolveMattermostReplyRootId", () => { }); it("falls back to undefined when neither reply target is available", () => { - expect(resolveMattermostReplyRootId({})).toBeUndefined(); + expect(resolveMattermostReplyRootId({ kind: "channel" })).toBeUndefined(); + }); + + it("keeps direct-message replies top-level even when a payload reply target exists", () => { + expect( + resolveMattermostReplyRootId({ + kind: "direct", + threadRootId: "dm-root-456", + replyToId: "dm-post-123", + }), + ).toBeUndefined(); + }); + + it("keeps direct-message replies top-level when only the payload reply target exists", () => { + expect( + resolveMattermostReplyRootId({ + kind: "direct", + replyToId: "dm-post-123", + }), + ).toBeUndefined(); + }); + + it("keeps group replies on the existing Mattermost thread root", () => { + expect( + resolveMattermostReplyRootId({ + kind: "group", + threadRootId: "group-root-456", + replyToId: "group-child-789", + }), + ).toBe("group-root-456"); }); }); @@ -206,6 +239,7 @@ describe("canFinalizeMattermostPreviewInPlace", () => { it("allows in-place finalization when the final reply target matches the preview thread", () => { expect( canFinalizeMattermostPreviewInPlace({ + kind: "channel", previewRootId: "thread-root-456", threadRootId: "thread-root-456", replyToId: "child-post-789", @@ -216,10 +250,20 @@ describe("canFinalizeMattermostPreviewInPlace", () => { it("prevents in-place finalization when a top-level preview would become a threaded reply", () => { expect( canFinalizeMattermostPreviewInPlace({ + kind: "channel", replyToId: "child-post-789", }), ).toBe(false); }); + + it("uses direct-message root suppression when checking in-place finalization", () => { + expect( + canFinalizeMattermostPreviewInPlace({ + kind: "direct", + replyToId: "dm-post-123", + }), + ).toBe(true); + }); }); describe("shouldClearMattermostDraftPreview", () => { @@ -259,6 +303,7 @@ describe("deliverMattermostReplyWithDraftPreview", () => { await deliverMattermostReplyWithDraftPreview({ payload: { text: " \n > Reasoning:\n> _hidden_" } as never, info: { kind: "final" }, + kind: "channel", client: createMattermostClientMock(), draftStream, effectiveReplyToId: "thread-root-1", @@ -282,6 +327,7 @@ describe("deliverMattermostReplyWithDraftPreview", () => { await deliverMattermostReplyWithDraftPreview({ payload: { text: "All good", replyToId: "reply-1" } as never, info: { kind: "final" }, + kind: "channel", client: createMattermostClientMock(), draftStream, resolvePreviewFinalText: (text) => text?.trim(), @@ -308,6 +354,7 @@ describe("deliverMattermostReplyWithDraftPreview", () => { mediaUrl: "https://example.com/a.png", } as never, info: { kind: "final" }, + kind: "channel", client: createMattermostClientMock(), draftStream, effectiveReplyToId: "thread-root-1", @@ -330,6 +377,7 @@ describe("deliverMattermostReplyWithDraftPreview", () => { await deliverMattermostReplyWithDraftPreview({ payload: { text: "Error", isError: true } as never, info: { kind: "final" }, + kind: "channel", client: createMattermostClientMock(), draftStream, effectiveReplyToId: "thread-root-1", @@ -351,6 +399,7 @@ describe("deliverMattermostReplyWithDraftPreview", () => { await deliverMattermostReplyWithDraftPreview({ payload: { text: "Final answer", replyToId: "child-post-789" } as never, info: { kind: "final" }, + kind: "channel", client: createMattermostClientMock(), draftStream, effectiveReplyToId: "thread-root-456", @@ -384,6 +433,7 @@ describe("deliverMattermostReplyWithDraftPreview", () => { deliverMattermostReplyWithDraftPreview({ payload: { text: "Broken", replyToId: "reply-1" } as never, info: { kind: "final" }, + kind: "channel", client: createMattermostClientMock(), draftStream, resolvePreviewFinalText: (text) => text?.trim(), @@ -484,6 +534,17 @@ describe("resolveMattermostEffectiveReplyToId", () => { }), ).toBeUndefined(); }); + + it("suppresses existing direct-message thread roots", () => { + expect( + resolveMattermostEffectiveReplyToId({ + kind: "direct", + postId: "post-123", + replyToMode: "all", + threadRootId: "dm-root-456", + }), + ).toBeUndefined(); + }); }); describe("resolveMattermostThreadSessionContext", () => { @@ -541,6 +602,7 @@ describe("resolveMattermostThreadSessionContext", () => { kind: "direct", postId: "post-123", replyToMode: "all", + threadRootId: "dm-root-456", }), ).toEqual({ effectiveReplyToId: undefined, diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 648bddf245e..948dcbaa709 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -41,6 +41,7 @@ import { import { evaluateMattermostMentionGate, mapMattermostChannelTypeToChatType, + resolveMattermostTrustedChatKind, } from "./monitor-gating.js"; import { formatInboundFromLabel, @@ -94,6 +95,7 @@ import { deactivateSlashCommands, getSlashCommandState } from "./slash-state.js" export { evaluateMattermostMentionGate, mapMattermostChannelTypeToChatType, + resolveMattermostTrustedChatKind, } from "./monitor-gating.js"; export type { MattermostMentionGateInput, @@ -231,9 +233,13 @@ function channelChatType(kind: ChatType): "direct" | "group" | "channel" { } export function resolveMattermostReplyRootId(params: { + kind: ChatType; threadRootId?: string; replyToId?: string; }): string | undefined { + if (params.kind === "direct") { + return undefined; + } const threadRootId = normalizeOptionalString(params.threadRootId); if (threadRootId) { return threadRootId; @@ -242,12 +248,14 @@ export function resolveMattermostReplyRootId(params: { } export function canFinalizeMattermostPreviewInPlace(params: { + kind: ChatType; previewRootId?: string; threadRootId?: string; replyToId?: string; }): boolean { return ( resolveMattermostReplyRootId({ + kind: params.kind, threadRootId: params.threadRootId, replyToId: params.replyToId, }) === params.previewRootId?.trim() @@ -275,6 +283,7 @@ type MattermostDraftPreviewState = { type MattermostDraftPreviewDeliverParams = { payload: ReplyPayload; info: { kind: "tool" | "block" | "final" }; + kind: ChatType; client: MattermostClient; draftStream: Pick< ReturnType, @@ -313,6 +322,7 @@ export async function deliverMattermostReplyWithDraftPreview( typeof previewFinalText !== "string" || payload.isError || !canFinalizeMattermostPreviewInPlace({ + kind: params.kind, previewRootId: params.effectiveReplyToId, threadRootId: params.effectiveReplyToId, replyToId: payload.replyToId, @@ -345,13 +355,13 @@ export function resolveMattermostEffectiveReplyToId(params: { replyToMode: "off" | "first" | "all" | "batched"; threadRootId?: string | null; }): string | undefined { + if (params.kind === "direct") { + return undefined; + } const threadRootId = normalizeOptionalString(params.threadRootId); if (threadRootId && params.replyToMode !== "off") { return threadRootId; } - if (params.kind === "direct") { - return undefined; - } const postId = normalizeOptionalString(params.postId); if (!postId) { return undefined; @@ -707,6 +717,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} accountId: account.accountId, agentId: route.agentId, replyToId: resolveMattermostReplyRootId({ + kind, threadRootId: threadContext.effectiveReplyToId, replyToId: payload.replyToId, }), @@ -915,6 +926,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} accountId: account.accountId, agentId: params.route.agentId, replyToId: resolveMattermostReplyRootId({ + kind: params.kind, threadRootId: params.effectiveReplyToId, replyToId: trimmedPayload.replyToId, }), @@ -1208,8 +1220,9 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} } const channelInfo = await resolveChannelInfo(channelId); - const channelType = payload.data?.channel_type ?? channelInfo?.type ?? undefined; - const kind = mapMattermostChannelTypeToChatType(channelType); + const kind = resolveMattermostTrustedChatKind({ + channelType: channelInfo?.type, + }); const chatType = channelChatType(kind); const senderName = @@ -1695,6 +1708,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} await deliverMattermostReplyWithDraftPreview({ payload, info, + kind, client, draftStream, effectiveReplyToId, @@ -1710,6 +1724,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} accountId: account.accountId, agentId: route.agentId, replyToId: resolveMattermostReplyRootId({ + kind, threadRootId: effectiveReplyToId, replyToId: payload.replyToId, }),