diff --git a/extensions/mattermost/src/__traces__/final-then-error.trace.jsonl b/extensions/mattermost/src/__traces__/final-then-error.trace.jsonl new file mode 100644 index 000000000000..7847c544d922 --- /dev/null +++ b/extensions/mattermost/src/__traces__/final-then-error.trace.jsonl @@ -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"} diff --git a/extensions/mattermost/src/delivery-trace.test.ts b/extensions/mattermost/src/delivery-trace.test.ts index b031f9506d55..0f7c89f29c50 100644 --- a/extensions/mattermost/src/delivery-trace.test.ts +++ b/extensions/mattermost/src/delivery-trace.test.ts @@ -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, }); }); diff --git a/extensions/mattermost/src/mattermost/monitor-draft-delivery.ts b/extensions/mattermost/src/mattermost/monitor-draft-delivery.ts index bea876734f15..cdd86fd681c0 100644 --- a/extensions/mattermost/src/mattermost/monitor-draft-delivery.ts +++ b/extensions/mattermost/src/mattermost/monitor-draft-delivery.ts @@ -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({ - 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); diff --git a/extensions/mattermost/src/mattermost/monitor.test.ts b/extensions/mattermost/src/mattermost/monitor.test.ts index 5515c386a2d4..e6fea71a70f0 100644 --- a/extensions/mattermost/src/mattermost/monitor.test.ts +++ b/extensions/mattermost/src/mattermost/monitor.test.ts @@ -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 () => {});