mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-02 11:11:35 +00:00
fix: preserve finalized Mattermost replies after tool warnings (#109555)
* fix(mattermost): preserve finalized preview after warnings * test(mattermost): cover finalized preview warning trace * docs(mattermost): clarify finalized preview invariant --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -0,0 +1,8 @@
|
||||
{"seq":1,"at":0,"dir":"in","kind":"reply-start"}
|
||||
{"seq":2,"at":0,"dir":"in","kind":"partial","data":{"text":"Successful assistant"}}
|
||||
{"seq":3,"at":0,"dir":"out","kind":"POST /posts","data":{"payload":{"channel_id":"channel-trace","message":"Successful assistant","root_id":"root-trace"},"result":{"id":"post-1"},"target":"channel-trace"}}
|
||||
{"seq":4,"at":1200,"dir":"in","kind":"final","data":{"text":"Successful assistant final"}}
|
||||
{"seq":5,"at":1200,"dir":"out","kind":"PUT /posts/post-1","data":{"payload":{"id":"post-1","message":"Successful assistant final"},"result":{"id":"post-1"},"target":"post-1"}}
|
||||
{"seq":6,"at":1200,"dir":"in","kind":"final","data":{"isError":true,"text":"Tool error warning"}}
|
||||
{"seq":7,"at":1200,"dir":"out","kind":"POST /posts","data":{"payload":{"channel_id":"channel-trace","message":"Tool error warning","root_id":"root-trace"},"result":{"id":"post-2"},"target":"channel-trace"}}
|
||||
{"seq":8,"at":1200,"dir":"in","kind":"idle"}
|
||||
@@ -12,7 +12,7 @@ import {
|
||||
expectDeliveryTraceMatchesGolden,
|
||||
runDeliveryTraceScenario,
|
||||
type DeliveryTraceInStep,
|
||||
type DeliveryTraceScenarioName,
|
||||
type DeliveryTraceScenario,
|
||||
type WireRecorder,
|
||||
} from "openclaw/plugin-sdk/channel-contract-testing";
|
||||
import type { OpenClawConfig, PluginRuntime } from "openclaw/plugin-sdk/core";
|
||||
@@ -251,21 +251,32 @@ function setupMattermostTrace(recorder: WireRecorder) {
|
||||
};
|
||||
}
|
||||
|
||||
const MATTERMOST_TRACE_SCENARIOS: readonly DeliveryTraceScenarioName[] = [
|
||||
"streaming-happy",
|
||||
"final-only",
|
||||
"cancel-mid-stream",
|
||||
const MATTERMOST_TRACE_SCENARIOS: readonly DeliveryTraceScenario[] = [
|
||||
deliveryTraceScenarios["streaming-happy"],
|
||||
deliveryTraceScenarios["final-only"],
|
||||
deliveryTraceScenarios["cancel-mid-stream"],
|
||||
{
|
||||
name: "final-then-error",
|
||||
steps: [
|
||||
{ kind: "reply-start" },
|
||||
{ kind: "partial", text: "Successful assistant" },
|
||||
{ kind: "advance", ms: DRAFT_THROTTLE_MS },
|
||||
{ kind: "final", text: "Successful assistant final" },
|
||||
{ kind: "final", text: "Tool error warning", isError: true },
|
||||
{ kind: "idle" },
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
describe("mattermost delivery trace goldens", () => {
|
||||
for (const scenarioName of MATTERMOST_TRACE_SCENARIOS) {
|
||||
it(`records ${scenarioName}`, async () => {
|
||||
for (const scenario of MATTERMOST_TRACE_SCENARIOS) {
|
||||
it(`records ${scenario.name}`, async () => {
|
||||
const events = await runDeliveryTraceScenario({
|
||||
scenario: deliveryTraceScenarios[scenarioName],
|
||||
scenario,
|
||||
setup: setupMattermostTrace,
|
||||
});
|
||||
expectDeliveryTraceMatchesGolden({
|
||||
goldenUrl: new URL(`./__traces__/${scenarioName}.trace.jsonl`, import.meta.url),
|
||||
goldenUrl: new URL(`./__traces__/${scenario.name}.trace.jsonl`, import.meta.url),
|
||||
events,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,6 +14,7 @@ import { canFinalizeMattermostPreviewInPlace } from "./monitor-context.js";
|
||||
import type { ChatType, ReplyPayload } from "./runtime-api.js";
|
||||
|
||||
export type MattermostDraftPreviewState = {
|
||||
/** True once the preview is the durable final post and must not be reused as a draft. */
|
||||
finalizedViaPreviewPost: boolean;
|
||||
};
|
||||
|
||||
@@ -48,13 +49,19 @@ export async function deliverMattermostReplyWithDraftPreview(
|
||||
kind: params.info.kind,
|
||||
payload: params.payload,
|
||||
adapter: defineFinalizableLivePreviewAdapter<ReplyPayload, string, { message: string }>({
|
||||
draft: {
|
||||
flush: params.draftStream.flush,
|
||||
clear: params.draftStream.clear,
|
||||
discardPending: params.draftStream.discardPending,
|
||||
seal: params.draftStream.seal,
|
||||
id: params.draftStream.postId,
|
||||
},
|
||||
// Once the preview is finalized, later payloads must use durable sends.
|
||||
// Reusing the sealed draft would clear and delete the successful final post.
|
||||
...(params.previewState.finalizedViaPreviewPost
|
||||
? {}
|
||||
: {
|
||||
draft: {
|
||||
flush: params.draftStream.flush,
|
||||
clear: params.draftStream.clear,
|
||||
discardPending: params.draftStream.discardPending,
|
||||
seal: params.draftStream.seal,
|
||||
id: params.draftStream.postId,
|
||||
},
|
||||
}),
|
||||
buildFinalEdit: (payload) => {
|
||||
const hasMedia = Boolean(payload.mediaUrl) || (payload.mediaUrls?.length ?? 0) > 0;
|
||||
const ttsSupplement = getReplyPayloadTtsSupplement(payload);
|
||||
|
||||
@@ -348,6 +348,40 @@ describe("deliverMattermostReplyWithDraftPreview", () => {
|
||||
expect(recordThreadParticipation).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("keeps a finalized preview when a later tool warning is delivered", async () => {
|
||||
const draftStream = createDraftStreamMock();
|
||||
const deliverFinal = vi.fn(async () => {});
|
||||
const previewState = { finalizedViaPreviewPost: false };
|
||||
const params = {
|
||||
kind: "direct" as const,
|
||||
client: createMattermostClientMock(),
|
||||
draftStream,
|
||||
resolvePreviewFinalText: (text?: string) => text?.trim(),
|
||||
previewState,
|
||||
logVerboseMessage: vi.fn(),
|
||||
deliverPayload: deliverFinal,
|
||||
};
|
||||
|
||||
await deliverMattermostReplyWithDraftPreview({
|
||||
...params,
|
||||
payload: { text: "Successful assistant final" } as never,
|
||||
info: { kind: "final" },
|
||||
});
|
||||
await deliverMattermostReplyWithDraftPreview({
|
||||
...params,
|
||||
payload: { text: "Tool error warning", isError: true } as never,
|
||||
info: { kind: "final" },
|
||||
});
|
||||
|
||||
expect(previewState.finalizedViaPreviewPost).toBe(true);
|
||||
expect(deliverFinal).toHaveBeenCalledExactlyOnceWith({
|
||||
text: "Tool error warning",
|
||||
isError: true,
|
||||
});
|
||||
expect(draftStream.discardPending).not.toHaveBeenCalled();
|
||||
expect(draftStream.clear).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("deletes the preview after a successful normal final send", async () => {
|
||||
const draftStream = createDraftStreamMock();
|
||||
const deliverFinal = vi.fn(async () => {});
|
||||
|
||||
Reference in New Issue
Block a user