ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-25 20:05:46 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(mattermost): preserve presentations in normal agent replies (#129579)

Browse Source
This commit is contained in:
Peter Steinberger
2026-08-25 14:46:28 -07:00
committed by GitHub
parent aaae1f0c8d
commit e1e2818a6a
6 changed files with 320 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/mattermost/src/channel.ts
+9 -46
View File
@@ -28,10 +28,7 @@ import { createChannelDirectoryAdapter } from "openclaw/plugin-sdk/directory-run
import { buildPassiveProbedChannelStatusSummary } from "openclaw/plugin-sdk/extension-shared";
import {
type MessagePresentation,
normalizeMessagePresentation,
renderMessagePresentationFallbackText,
resolveMessagePresentationButtonAction,
resolveMessagePresentationControlValue,
} from "openclaw/plugin-sdk/interactive-runtime";
import { createLazyRuntimeModule } from "openclaw/plugin-sdk/lazy-runtime";
import { resolvePayloadMediaUrls, sendTextMediaPayload } from "openclaw/plugin-sdk/reply-payload";
@@ -73,6 +70,7 @@ import type { MattermostSendResult } from "./mattermost/send.js";
import {
looksLikeMattermostTargetId,
normalizeMattermostMessagingTarget,
resolveMattermostPresentation,
requiresMattermostMediaUpload,
} from "./normalize.js";
import { collectRuntimeConfigAssignments, secretTargetRegistryEntries } from "./secret-contract.js";
@@ -83,33 +81,6 @@ import type { MattermostConfig } from "./types.js";
const loadMattermostChannelRuntime = createLazyRuntimeModule(() => import("./channel.runtime.js"));
function buildMattermostPresentationButtons(presentation: MessagePresentation) {
return presentation.blocks
.filter((block) => block.type === "buttons")
.map((block) =>
block.buttons.flatMap((button) => {
if (button.action) {
return [];
}
const value = resolveMessagePresentationControlValue(button);
return value
? [
{
id: value,
text: button.label,
callback_data: value,
context: {
callback_data: value,
},
style: button.style,
},
]
: [];
}),
)
.filter((row) => row.length > 0);
}
const MATTERMOST_PRESENTATION_CAPABILITIES = {
supported: true,
buttons: true,
@@ -572,15 +543,10 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
throw new Error("Mattermost send requires a target (to).");
}
const presentation = normalizeMessagePresentation(params.presentation);
const message = presentation
? renderMessagePresentationFallbackText({
text: typeof params.message === "string" ? params.message : "",
presentation,
})
: typeof params.message === "string"
? params.message
: "";
const { text: message, buttons } = resolveMattermostPresentation({
text: typeof params.message === "string" ? params.message : undefined,
presentation: params.presentation,
});
// Mattermost post root_id is the thread root. A generic replyTo can name
// the current child post, so prefer threadId unless the caller supplied the
// Mattermost-specific replyToId root directly.
@@ -591,8 +557,6 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
const resolvedAccountId = accountId || undefined;
const mediaUrl = resolveMattermostSendAttachmentMedia(params);
const buttons = presentation ? buildMattermostPresentationButtons(presentation) : [];
const result = await (
await loadMattermostChannelRuntime()
).sendMessageMattermost(to, message, {
@@ -803,15 +767,14 @@ const mattermostOutbound: ChannelOutboundAdapter = {
if (payload.mediaUrls && payload.mediaUrls.length > 1) {
return null;
}
const buttons = buildMattermostPresentationButtons(presentation);
const hasButtons = buttons.some((row) => row.length > 0);
if (!hasButtons && !hasMattermostPresentationNavigation(presentation)) {
const { text, buttons } = resolveMattermostPresentation({ text: payload.text, presentation });
if (!buttons.length && !hasMattermostPresentationNavigation(presentation)) {
return null;
}
return {
...payload,
text: renderMessagePresentationFallbackText({ text: payload.text, presentation }),
...(hasButtons
text,
...(buttons.length
? {
channelData: {
...payload.channelData,
extensions/mattermost/src/mattermost/monitor-draft-delivery.test.ts
+55
View File
@@ -147,6 +147,41 @@ describe("deliverMattermostReplyWithDraftPreview", () => {
});
});
it.each([
{
name: "native value buttons",
presentation: {
blocks: [{ type: "buttons" as const, buttons: [{ label: "Open", value: "open" }] }],
},
},
{
name: "navigation URLs",
presentation: {
blocks: [
{
type: "buttons" as const,
buttons: [{ label: "Docs", url: "https://example.com/docs" }],
},
],
},
},
])("delivers $name instead of losing them in a preview edit", async ({ presentation }) => {
const draftStream = createDraftStreamMock();
const deliverFinal = createDeliverFinalMock();
const payload = { text: "Choose an option", presentation };
await deliverDraftPreview({
payload,
draftStream,
effectiveReplyToId: "thread-root-1",
deliverPayload: deliverFinal,
});
expect(deliverFinal).toHaveBeenCalledExactlyOnceWith(payload);
expect(updateMattermostPostSpy).not.toHaveBeenCalled();
expect(draftStream.clear).toHaveBeenCalledTimes(1);
});
it("reports a final already published in a sealed preview generation", async () => {
const draftStream = createDraftStreamMock(null);
const deliverFinal = createDeliverFinalMock();
@@ -174,6 +209,26 @@ describe("deliverMattermostReplyWithDraftPreview", () => {
});
});
it("delivers unsent presentation controls after preview text was already published", async () => {
const draftStream = createDraftStreamMock(null);
const deliverFinal = createDeliverFinalMock();
const confirmedDelivery = createConfirmedPreviewDelivery("sealed-post-1", "Already visible");
const presentation = {
blocks: [{ type: "buttons" as const, buttons: [{ label: "Open", value: "open" }] }],
};
const result = await deliverDraftPreview({
payload: { text: "Already visible", presentation },
draftStream,
effectiveReplyToId: "thread-root-1",
resolvePreviewFinalText: () => ({ alreadyDelivered: true, confirmedDelivery }),
deliverPayload: deliverFinal,
});
expect(deliverFinal).toHaveBeenCalledExactlyOnceWith({ text: "", presentation });
expect(result.messageIds).toEqual(["sealed-post-1", "delivered-post-1"]);
});
it("still delivers media when the text is already published", async () => {
const draftStream = createDraftStreamMock(null);
const confirmedDelivery = createConfirmedPreviewDelivery("sealed-post-1", "Already visible");
extensions/mattermost/src/mattermost/monitor-draft-delivery.ts
+4 -1
View File
@@ -134,15 +134,18 @@ export async function deliverMattermostReplyWithDraftPreview(
previewFinalDeliveryText = previewFinalResolution?.deliveryText;
previewFinalTextAlreadyDelivered =
previewFinalResolution?.alreadyDelivered === true && payload.isError !== true;
// A text-only preview cannot finalize unsent presentation content or controls.
useConfirmedPreviewAsWholeFinal =
previewFinalTextAlreadyDelivered &&
!resolveSendableOutboundReplyParts(payload).hasMedia;
!resolveSendableOutboundReplyParts(payload).hasMedia &&
!payload.presentation;
const previewFinalText = previewFinalResolution?.editText;
if (
(hasMedia && !ttsSupplement) ||
typeof previewFinalText !== "string" ||
payload.isError ||
payload.presentation ||
!canFinalizeMattermostPreviewInPlace({
kind: params.kind,
previewRootId: params.effectiveReplyToId,
extensions/mattermost/src/mattermost/reply-delivery.test.ts
+189 -3
View File
@@ -2,7 +2,7 @@
import path from "node:path";
import { isChannelPartialDeliveryError } from "openclaw/plugin-sdk/channel-inbound";
import { createMessageReceiptFromOutboundResults } from "openclaw/plugin-sdk/channel-outbound";
import type { ChunkMode } from "openclaw/plugin-sdk/reply-runtime";
import type { ChunkMode, ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
import { createOpenClawTestState } from "openclaw/plugin-sdk/test-state";
import { describe, expect, it, vi } from "vitest";
import type { OpenClawConfig, PluginRuntime } from "../../runtime-api.js";
@@ -49,7 +49,7 @@ function createSendMessageMock() {
async (
_to: string,
content: string,
_opts: SendMattermostMessageOptions,
opts: SendMattermostMessageOptions,
): Promise<MattermostSendResult> => {
const messageId = `post-${++sendCount}`;
return {
@@ -58,7 +58,10 @@ function createSendMessageMock() {
content: content.trim(),
receipt: createMessageReceiptFromOutboundResults({
results: [{ channel: "mattermost", messageId, channelId: "channel-1" }],
kind: "text",
kind:
"buttons" in opts && Array.isArray(opts.buttons) && opts.buttons.length
? "card"
: "text",
}),
};
},
@@ -66,6 +69,189 @@ function createSendMessageMock() {
}
describe("deliverMattermostReplyPayload", () => {
it.each<{
name: string;
payload: ReplyPayload;
expectedText: string;
expectedButtonValue?: string;
}>([
{
name: "presentation-only title",
payload: { presentation: { title: "Build complete", blocks: [] } },
expectedText: "Build complete",
},
{
name: "presentation-only text",
payload: { presentation: { blocks: [{ type: "text", text: "Release finished" }] } },
expectedText: "Release finished",
},
{
name: "presentation-only native value button",
payload: {
presentation: {
blocks: [{ type: "buttons", buttons: [{ label: "Open", value: "open" }] }],
},
},
expectedText: "- Open",
expectedButtonValue: "open",
},
{
name: "presentation-only URL button",
payload: {
presentation: {
blocks: [
{
type: "buttons",
buttons: [{ label: "Docs", url: "https://example.com/docs" }],
},
],
},
},
expectedText: "- Docs: https://example.com/docs",
},
{
name: "presentation-only select",
payload: {
presentation: {
blocks: [
{
type: "select",
placeholder: "Environment",
options: [{ label: "Production", value: "production" }],
},
],
},
},
expectedText: "Environment:\n- Production",
},
{
name: "authored text with native buttons",
payload: {
text: "Deploy finished",
presentation: {
blocks: [{ type: "buttons", buttons: [{ label: "Open", value: "open" }] }],
},
},
expectedText: "Deploy finished\n\n- Open",
expectedButtonValue: "open",
},
{
name: "authored fallback without duplicate presentation text",
payload: {
text: "Already formatted",
presentationTextMode: "fallback",
presentation: { blocks: [{ type: "text", text: "Generated summary" }] },
},
expectedText: "Already formatted",
},
{
name: "typed callback action stays text-only",
payload: {
presentation: {
blocks: [
{
type: "buttons",
buttons: [
{
label: "Inspect",
action: { type: "callback", value: "private-callback-token" },
},
],
},
],
},
},
expectedText: "- Inspect",
},
])(
"delivers $name through the normal reply owner",
async ({ payload, expectedText, expectedButtonValue }) => {
const sendMessage = createSendMessageMock();
const result = await deliverMattermostReplyPayload({
core: createReplyDeliveryCore(),
cfg: {},
payload,
channelId: "town-square",
accountId: "default",
replyToId: "root-post",
textLimit: 4000,
tableMode: "off",
sendMessage,
});
expect(sendMessage).toHaveBeenCalledTimes(1);
expect(sendMessage).toHaveBeenCalledWith(
"channel:town-square",
expectedText,
expect.objectContaining({ accountId: "default", replyToId: "root-post" }),
);
const options = sendMessage.mock.calls[0]![2];
if (expectedButtonValue) {
expect(options).toMatchObject({
buttons: [[{ id: expectedButtonValue, callback_data: expectedButtonValue }]],
});
} else {
expect("buttons" in options ? options.buttons : undefined).toBeUndefined();
}
expect(result).toMatchObject({ outcome: "text", visibleReplySent: true });
expect(result.receipt?.parts[0]?.kind).toBe(expectedButtonValue ? "card" : "text");
},
);
it("attaches presentation controls only to the first visible text chunk", async () => {
const sendMessage = createSendMessageMock();
const core = createReplyDeliveryCore();
core.channel.text.chunkMarkdownTextWithMode = vi.fn(() => ["alpha", "beta"]);
await deliverMattermostReplyPayload({
core,
cfg: {},
payload: {
text: "alpha beta",
presentation: {
blocks: [{ type: "buttons", buttons: [{ label: "Open", value: "open" }] }],
},
},
channelId: "town-square",
accountId: "default",
textLimit: 6,
tableMode: "off",
sendMessage,
});
expect(sendMessage).toHaveBeenCalledTimes(2);
expect(sendMessage.mock.calls[0]![2]).toMatchObject({
buttons: [[{ id: "open", callback_data: "open" }]],
});
expect("buttons" in sendMessage.mock.calls[1]![2]).toBe(false);
});
it("keeps multiple-media presentations on the text/media fallback path", async () => {
const sendMessage = createSendMessageMock();
await deliverMattermostReplyPayload({
core: createReplyDeliveryCore(),
cfg: {},
payload: {
mediaUrls: ["https://example.com/1.png", "https://example.com/2.png"],
presentation: {
blocks: [{ type: "buttons", buttons: [{ label: "Open", value: "open" }] }],
},
},
channelId: "town-square",
accountId: "default",
textLimit: 4000,
tableMode: "off",
sendMessage,
});
expect(sendMessage).toHaveBeenCalledTimes(2);
expect(sendMessage.mock.calls[0]![1]).toBe("- Open");
for (const call of sendMessage.mock.calls) {
expect("buttons" in call[2]).toBe(false);
}
});
it("suppresses payloads flagged as reasoning", async () => {
const sendMessage = createSendMessageMock();
const cfg = {} satisfies OpenClawConfig;
extensions/mattermost/src/mattermost/reply-delivery.ts
+21 -29
View File
@@ -16,7 +16,7 @@ import {
resolveSendableOutboundReplyParts,
} from "openclaw/plugin-sdk/reply-payload";
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
import { requiresMattermostMediaUpload } from "../normalize.js";
import { requiresMattermostMediaUpload, resolveMattermostPresentation } from "../normalize.js";
import type { MattermostSendResult } from "./send.js";
type MarkdownTableMode = Parameters<PluginRuntime["channel"]["text"]["convertMarkdownTables"]>[1];
@@ -31,6 +31,7 @@ type SendMattermostMessage = (
mediaLocalRoots?: readonly string[];
requireMediaUpload?: boolean;
replyToId?: string;
buttons?: Array<unknown>;
},
) => Promise<MattermostSendResult>;
@@ -75,11 +76,9 @@ export async function deliverMattermostReplyPayload(params: {
suppression: { reason: "no_visible_result" },
};
}
const presentation = resolveMattermostPresentation(params.payload);
const reply = resolveSendableOutboundReplyParts(params.payload, {
text: params.core.channel.text.convertMarkdownTables(
params.payload.text ?? "",
params.tableMode,
),
text: params.core.channel.text.convertMarkdownTables(presentation.text, params.tableMode),
});
const mediaLocalRoots = getAgentScopedMediaLocalRoots(params.cfg, params.agentId);
const chunkMode = params.core.channel.text.resolveChunkMode(
@@ -89,7 +88,21 @@ export async function deliverMattermostReplyPayload(params: {
);
const results: MattermostSendResult[] = [];
const acceptedContents: string[] = [];
const deliveryTarget = `channel:${params.channelId}`;
const sendAccepted = async (text: string, mediaUrl?: string) => {
const result = await params.sendMessage(`channel:${params.channelId}`, text, {
cfg: params.cfg,
accountId: params.accountId,
...(mediaUrl ? { mediaUrl, mediaLocalRoots } : {}),
// Local media must upload successfully instead of silently posting only its caption.
...(requiresMattermostMediaUpload(mediaUrl) ? { requireMediaUpload: true } : {}),
...(results.length === 0 && reply.mediaUrls.length < 2 && presentation.buttons.length
? { buttons: presentation.buttons }
: {}),
replyToId: params.replyToId,
});
results.push(result);
acceptedContents.push(result.content);
};
let outcome: Exclude<MattermostReplyDeliveryOutcome, "reasoning_skipped">;
try {
outcome = await deliverTextOrMediaReply({
@@ -97,29 +110,8 @@ export async function deliverMattermostReplyPayload(params: {
text: reply.text,
chunkText: (value) =>
params.core.channel.text.chunkMarkdownTextWithMode(value, params.textLimit, chunkMode),
sendText: async (chunk) => {
const result = await params.sendMessage(deliveryTarget, chunk, {
cfg: params.cfg,
accountId: params.accountId,
replyToId: params.replyToId,
});
results.push(result);
acceptedContents.push(result.content);
},
sendMedia: async ({ mediaUrl, caption }) => {
// Require upload for local media so a failure surfaces instead of
// silently posting the caption alone (mirrors channel.ts send paths).
const result = await params.sendMessage(deliveryTarget, caption ?? "", {
cfg: params.cfg,
accountId: params.accountId,
mediaUrl,
mediaLocalRoots,
...(requiresMattermostMediaUpload(mediaUrl) ? { requireMediaUpload: true } : {}),
replyToId: params.replyToId,
});
results.push(result);
acceptedContents.push(result.content);
},
sendText: sendAccepted,
sendMedia: ({ mediaUrl, caption }) => sendAccepted(caption ?? "", mediaUrl),
});
} catch (error: unknown) {
const failedPartial = isChannelPartialDeliveryError(error) ? error.deliveryResult : undefined;
extensions/mattermost/src/normalize.ts
+42
View File
@@ -1,6 +1,48 @@
// Mattermost helper module supports normalize behavior.
import {
normalizeMessagePresentation,
renderMessagePresentationFallbackText,
resolveMessagePresentationControlValue,
} from "openclaw/plugin-sdk/interactive-runtime";
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime";
export function resolveMattermostPresentation(params: {
text?: string;
presentation?: unknown;
presentationTextMode?: "fallback";
}) {
const presentation = normalizeMessagePresentation(params.presentation);
const text =
!presentation || (params.presentationTextMode === "fallback" && params.text !== undefined)
? (params.text ?? "")
: renderMessagePresentationFallbackText({ text: params.text, presentation });
const buttons = presentation
? presentation.blocks
.filter((block) => block.type === "buttons")
.map((block) =>
block.buttons.flatMap((button) => {
if (button.action) {
return [];
}
const value = resolveMessagePresentationControlValue(button);
return value
? [
{
id: value,
text: button.label,
callback_data: value,
context: { callback_data: value },
style: button.style,
},
]
: [];
}),
)
.filter((row) => row.length > 0)
: [];
return { text, buttons };
}
export function normalizeMattermostMessagingTarget(raw: string): string | undefined {
const trimmed = raw.trim();
if (!trimmed) {