fix(mattermost): gate delivery success log

This commit is contained in:
Peter Steinberger
2026-05-14 08:13:04 +01:00
parent 6789fe248b
commit 643dea2455
5 changed files with 113 additions and 27 deletions

View File

@@ -9,6 +9,7 @@ import {
canFinalizeMattermostPreviewInPlace,
deliverMattermostReplyWithDraftPreview,
evaluateMattermostMentionGate,
formatMattermostFinalDeliveryOutcomeLog,
MattermostRetryableInboundError,
processMattermostReplayGuardedPost,
resolveMattermostReactionChannelId,
@@ -555,6 +556,74 @@ describe("deliverMattermostReplyWithDraftPreview", () => {
});
});
describe("formatMattermostFinalDeliveryOutcomeLog", () => {
it("logs delivered only for visible text and media outcomes", () => {
expect(
formatMattermostFinalDeliveryOutcomeLog({
outcome: "text",
payload: { text: "hello" } as never,
to: "channel:town-square",
accountId: "default",
agentId: "agent-1",
}),
).toBe("delivered reply to channel:town-square");
expect(
formatMattermostFinalDeliveryOutcomeLog({
outcome: "media",
payload: { mediaUrl: "https://example.com/a.png" } as never,
to: "channel:town-square",
accountId: "default",
agentId: "agent-1",
}),
).toBe("delivered reply to channel:town-square");
});
it("does not log delivered for empty no-send outcomes without diagnostic violations", () => {
expect(
formatMattermostFinalDeliveryOutcomeLog({
outcome: "empty",
payload: { text: " \n\t " } as never,
to: "channel:town-square",
accountId: "default",
agentId: "agent-1",
}),
).toBeUndefined();
});
it("logs a diagnostic for substantive empty outcomes", () => {
expect(
formatMattermostFinalDeliveryOutcomeLog({
outcome: "empty",
payload: { text: "work result" } as never,
to: "channel:town-square",
accountId: "default",
agentId: "agent-1",
}),
).toBe(
"mattermost no-visible-reply: no-visible-reply-after-final-delivery" +
" to=channel:town-square" +
" accountId=default" +
" agentId=agent-1" +
" outcome=empty" +
" finalTextLength=11" +
" mediaUrlCount=0",
);
});
it("does not log reasoning-suppressed outcomes", () => {
expect(
formatMattermostFinalDeliveryOutcomeLog({
outcome: "reasoning_skipped",
payload: { text: "Reasoning: hidden" } as never,
to: "channel:town-square",
accountId: "default",
agentId: "agent-1",
}),
).toBeUndefined();
});
});
describe("shouldFinalizeMattermostPreviewAfterDispatch", () => {
it("reuses the preview only for a single eligible final payload", () => {
expect(

View File

@@ -71,7 +71,10 @@ import {
formatMattermostNoVisibleReplyLog,
} from "./no-visible-reply-diagnostic.js";
import { runWithReconnect } from "./reconnect.js";
import { deliverMattermostReplyPayload } from "./reply-delivery.js";
import {
deliverMattermostReplyPayload,
type MattermostReplyDeliveryOutcome,
} from "./reply-delivery.js";
import type {
ChannelAccountSnapshot,
ChatType,
@@ -383,6 +386,31 @@ export async function deliverMattermostReplyWithDraftPreview(
});
}
export function formatMattermostFinalDeliveryOutcomeLog(params: {
outcome: MattermostReplyDeliveryOutcome;
payload: ReplyPayload;
to: string;
accountId: string;
agentId: string | undefined;
}): string | undefined {
const violation = evaluateMattermostNoVisibleReply({
outcome: params.outcome,
payload: params.payload,
});
if (violation) {
return formatMattermostNoVisibleReplyLog({
violation,
to: params.to,
accountId: params.accountId,
agentId: params.agentId,
});
}
if (params.outcome === "text" || params.outcome === "media") {
return `delivered reply to ${params.to}`;
}
return undefined;
}
export function resolveMattermostEffectiveReplyToId(params: {
kind: ChatType;
postId?: string | null;
@@ -1700,18 +1728,15 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
tableMode,
sendMessage: sendMessageMattermost,
});
const violation = evaluateMattermostNoVisibleReply({ outcome, payload });
if (violation) {
runtime.log?.(
formatMattermostNoVisibleReplyLog({
violation,
to,
accountId: account.accountId,
agentId: route.agentId,
}),
);
} else if (outcome !== "reasoning_skipped") {
runtime.log?.(`delivered reply to ${to}`);
const deliveryLog = formatMattermostFinalDeliveryOutcomeLog({
outcome,
payload,
to,
accountId: account.accountId,
agentId: route.agentId,
});
if (deliveryLog) {
runtime.log?.(deliveryLog);
}
},
});

View File

@@ -31,7 +31,7 @@ describe("evaluateMattermostNoVisibleReply", () => {
});
});
it("counts entries from mediaUrls[] alongside the singular mediaUrl", () => {
it("follows the SDK legacy media fallback when counting media URLs", () => {
const violation = evaluateMattermostNoVisibleReply({
outcome: "empty",
payload: {
@@ -39,7 +39,7 @@ describe("evaluateMattermostNoVisibleReply", () => {
mediaUrls: ["https://example.org/b.png", "https://example.org/c.png"],
},
});
expect(violation?.mediaUrlCount).toBe(3);
expect(violation?.mediaUrlCount).toBe(2);
});
it("does not flag reasoning_skipped outcome (intentional suppression)", () => {

View File

@@ -1,3 +1,4 @@
import { countOutboundMedia } from "openclaw/plugin-sdk/reply-payload";
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
import type { MattermostReplyDeliveryOutcome } from "./reply-delivery.js";
@@ -8,14 +9,6 @@ export type MattermostNoVisibleReplyViolation = {
mediaUrlCount: number;
};
function countMediaUrls(payload: ReplyPayload): number {
const single = typeof payload.mediaUrl === "string" && payload.mediaUrl.length > 0 ? 1 : 0;
const list = Array.isArray(payload.mediaUrls)
? payload.mediaUrls.filter((url) => typeof url === "string" && url.length > 0).length
: 0;
return single + list;
}
/**
* Detects the #80501 symptom: `deliverMattermostReplyPayload` accepted a
* substantive (non-reasoning) payload, called the underlying
@@ -38,7 +31,7 @@ export function evaluateMattermostNoVisibleReply(params: {
return null;
}
const finalText = typeof params.payload.text === "string" ? params.payload.text.trim() : "";
const mediaUrlCount = countMediaUrls(params.payload);
const mediaUrlCount = countOutboundMedia(params.payload);
// If the payload had no text and no media even nominally, the run had
// nothing to send and "empty" is the correct outcome — do not flag.
if (finalText.length === 0 && mediaUrlCount === 0) {

View File

@@ -190,14 +190,13 @@ describe("deliverMattermostReplyPayload", () => {
accountId: "default",
mediaUrl,
replyToId: "root-post",
mediaLocalRoots: [
path.join(os.tmpdir(), "openclaw"),
mediaLocalRoots: expect.arrayContaining([
path.join(stateDir, "media"),
path.join(stateDir, "canvas"),
path.join(stateDir, "workspace"),
path.join(stateDir, "sandboxes"),
path.join(stateDir, `workspace-${agentId}`),
],
]),
});
} finally {
if (previousStateDir === undefined) {