From 730cf5691599bf66943befd533fb0da20e6df29d Mon Sep 17 00:00:00 2001 From: Ahmed Tarek Date: Sat, 1 Aug 2026 08:32:56 +0100 Subject: [PATCH] fix(msteams): prevent duplicate final replies (#116398) * fix(msteams): preserve message-tool thread routing Co-authored-by: Vincent Koc * test(msteams): prove thread-aware final dedupe * fix(agents): suppress duplicate source previews * fix(agents): dedupe current-source reply previews --------- Co-authored-by: Vincent Koc --- CHANGELOG.md | 1 + .../msteams/src/action-threading.test.ts | 184 +++++++++++ extensions/msteams/src/action-threading.ts | 145 +++++++++ .../msteams/src/channel.actions.test.ts | 286 ++++++++++++++++++ .../src/channel.message-adapter.test.ts | 35 +++ extensions/msteams/src/channel.ts | 26 +- .../src/monitor-handler/inbound-dispatch.ts | 6 + .../message-handler.authz.test.ts | 2 + extensions/msteams/src/outbound.test.ts | 60 ++++ extensions/msteams/src/outbound.ts | 49 ++- .../msteams/src/qa/adapter.runtime.test.ts | 10 + extensions/msteams/src/qa/adapter.runtime.ts | 6 +- extensions/msteams/src/send-context.test.ts | 62 ++++ extensions/msteams/src/send-context.ts | 39 ++- .../mock-openai/mock-openai-contracts.ts | 1 + .../src/providers/mock-openai/server.test.ts | 38 +++ .../src/providers/mock-openai/server.ts | 13 + .../src/scenario-catalog-channels.test.ts | 20 ++ ...eams-thread-message-tool-final-dedupe.yaml | 243 +++++++++++++++ .../agent-tool-handler-state.test-helpers.ts | 1 + ...ed-agent-message-tool-source-reply.test.ts | 34 +++ ...mbedded-agent-message-tool-source-reply.ts | 23 +- ...ubscribe.handlers.messages.test-support.ts | 25 ++ ...-agent-subscribe.handlers.messages.test.ts | 133 ++++++++ ...edded-agent-subscribe.handlers.messages.ts | 48 ++- ...ent-subscribe.handlers.tools.media.test.ts | 1 + ...ded-agent-subscribe.handlers.tools.test.ts | 125 ++++++++ ...embedded-agent-subscribe.handlers.tools.ts | 34 ++- ...embedded-agent-subscribe.handlers.types.ts | 3 + src/agents/embedded-agent-subscribe.ts | 14 + src/auto-reply/reply/reply-payloads.test.ts | 32 ++ 31 files changed, 1665 insertions(+), 34 deletions(-) create mode 100644 extensions/msteams/src/action-threading.test.ts create mode 100644 extensions/msteams/src/action-threading.ts create mode 100644 qa/scenarios/channels/msteams-thread-message-tool-final-dedupe.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index f3c09d2bcf08..f5173467670c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- **Microsoft Teams message-tool replies:** keep automatic live previews from duplicating a message already delivered to the current Teams conversation, while preserving distinct follow-up text and cross-conversation sends. Fixes #116397. (#116398) Thanks @a-tokyo. - **Buzz plugin packaging:** keep the live QA runner on the shipped QA runner SDK surface and remove the obsolete package shrinkwrap so standalone npm and ClawHub package builds use current host exports and dependency resolutions. Thanks @shakkernerd. - **Control UI sharing connection isolation:** discard stale visibility and membership mutation results after switching gateways or accounts so previous-connection refreshes and errors cannot update the replacement connection. Fixes #116800. Thanks @shakkernerd. - **Control UI session refreshes:** preserve explicitly queued list filters and background hydration across later Gateway event invalidation, while keeping append pagination followed by a canonical refresh. Fixes #116697. Thanks @shakkernerd. diff --git a/extensions/msteams/src/action-threading.test.ts b/extensions/msteams/src/action-threading.test.ts new file mode 100644 index 000000000000..0e839af9c11f --- /dev/null +++ b/extensions/msteams/src/action-threading.test.ts @@ -0,0 +1,184 @@ +import { describe, expect, it } from "vitest"; +import { msteamsContextTargetsMatch, resolveMSTeamsAutoThreadId } from "./action-threading.js"; + +describe("msteamsContextTargetsMatch", () => { + it("matches conversation: targets against currentChannelId", () => { + expect( + msteamsContextTargetsMatch("conversation:19:channel@thread.tacv2", { + currentChannelId: "conversation:19:channel@thread.tacv2", + }), + ).toBe(true); + }); + + it("matches bare conversation id against conversation: currentChannelId", () => { + expect( + msteamsContextTargetsMatch("19:channel@thread.tacv2", { + currentChannelId: "conversation:19:channel@thread.tacv2", + }), + ).toBe(true); + }); + + it("matches when one side includes ;messageid=", () => { + expect( + msteamsContextTargetsMatch("conversation:19:channel@thread.tacv2;messageid=abc", { + currentChannelId: "conversation:19:channel@thread.tacv2", + }), + ).toBe(true); + }); + + it("rejects a different conversation", () => { + expect( + msteamsContextTargetsMatch("conversation:19:other@thread.tacv2", { + currentChannelId: "conversation:19:channel@thread.tacv2", + }), + ).toBe(false); + }); + + it("keeps opaque conversation ids case-sensitive", () => { + expect( + msteamsContextTargetsMatch("conversation:19:Channel@thread.tacv2", { + currentChannelId: "conversation:19:channel@thread.tacv2", + }), + ).toBe(false); + }); + + it("matches Graph team/channel messaging targets", () => { + expect( + msteamsContextTargetsMatch("team-1/19:channel@thread.tacv2", { + currentMessagingTarget: "team-1/19:channel@thread.tacv2", + }), + ).toBe(true); + }); + + it("matches Graph targets when one side includes ;messageid=", () => { + expect( + msteamsContextTargetsMatch("team-1/19:channel@thread.tacv2;messageid=root", { + currentMessagingTarget: "team-1/19:channel@thread.tacv2", + }), + ).toBe(true); + }); + + it("keeps opaque Graph targets case-sensitive", () => { + expect( + msteamsContextTargetsMatch("team-1/19:Channel@thread.tacv2", { + currentMessagingTarget: "team-1/19:channel@thread.tacv2", + }), + ).toBe(false); + }); + + it("rejects Graph messaging target against conversation channel id", () => { + expect( + msteamsContextTargetsMatch("team-1/19:channel@thread.tacv2", { + currentChannelId: "conversation:19:channel@thread.tacv2", + }), + ).toBe(false); + }); +}); + +describe("resolveMSTeamsAutoThreadId", () => { + const sameChannel = { + currentChannelId: "conversation:19:channel@thread.tacv2", + currentThreadTs: "thread-root", + replyToMode: "all" as const, + }; + + it("returns ambient thread root for same conversation", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:channel@thread.tacv2", + toolContext: sameChannel, + }), + ).toBe("thread-root"); + }); + + it("returns ambient thread root for bare conversation id target", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "19:channel@thread.tacv2", + toolContext: sameChannel, + }), + ).toBe("thread-root"); + }); + + it("returns ambient thread root for matching Graph messaging target", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "team-1/19:channel@thread.tacv2", + toolContext: { + currentMessagingTarget: "team-1/19:channel@thread.tacv2", + currentThreadTs: "thread-root", + replyToMode: "all", + }, + }), + ).toBe("thread-root"); + }); + + it("preserves an explicit message id instead of the ambient thread root", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:channel@thread.tacv2;messageid=explicit-root", + toolContext: sameChannel, + }), + ).toBe("explicit-root"); + }); + + it("preserves an explicit message id without ambient tool context", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:channel@thread.tacv2;messageid=explicit-root", + }), + ).toBe("explicit-root"); + }); + + it("preserves an explicit message id outside the ambient conversation", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:other@thread.tacv2;messageid=explicit-root", + toolContext: sameChannel, + }), + ).toBe("explicit-root"); + }); + + it("returns undefined for a different conversation", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:other@thread.tacv2", + toolContext: sameChannel, + }), + ).toBeUndefined(); + }); + + it("returns undefined when replyToMode is off", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:channel@thread.tacv2", + toolContext: { ...sameChannel, replyToMode: "off" }, + }), + ).toBeUndefined(); + }); + + it("returns undefined after a single-use reply when already replied", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:channel@thread.tacv2", + toolContext: { + ...sameChannel, + replyToMode: "first", + hasRepliedRef: { value: true }, + }, + }), + ).toBeUndefined(); + }); + + it("returns undefined when currentThreadTs is missing", () => { + expect( + resolveMSTeamsAutoThreadId({ + to: "conversation:19:channel@thread.tacv2", + toolContext: { + currentChannelId: "conversation:19:channel@thread.tacv2", + replyToMode: "all", + }, + }), + ).toBeUndefined(); + }); +}); diff --git a/extensions/msteams/src/action-threading.ts b/extensions/msteams/src/action-threading.ts new file mode 100644 index 000000000000..c2052f218a5d --- /dev/null +++ b/extensions/msteams/src/action-threading.ts @@ -0,0 +1,145 @@ +// MS Teams plugin module implements action threading behavior. +import type { ChannelToolSend } from "openclaw/plugin-sdk/channel-contract"; +import { isSingleUseReplyToMode } from "openclaw/plugin-sdk/reply-reference"; +import { isRecord, normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; +import type { MSTeamsConfig } from "../runtime-api.js"; +import { extractMSTeamsConversationMessageId, normalizeMSTeamsConversationId } from "./inbound.js"; +import { resolveMSTeamsReplyPolicy, resolveMSTeamsRouteConfig } from "./policy.js"; +import { parseMSTeamsTeamChannelInput } from "./resolve-allowlist.js"; + +function stripConversationPrefix(raw: string): string { + const trimmed = raw.trim(); + if (/^conversation:/i.test(trimmed)) { + return trimmed.slice("conversation:".length).trim(); + } + return trimmed; +} + +/** Normalize Teams conversation targets for equality (strips `conversation:` and `;messageid=`). */ +function normalizeMSTeamsThreadingTarget(raw: string | undefined): string | undefined { + const value = normalizeOptionalString(raw); + if (!value) { + return undefined; + } + return normalizeMSTeamsConversationId(stripConversationPrefix(value)); +} + +function extractMSTeamsResultConversationId(value: unknown): string | undefined { + if (!isRecord(value)) { + return undefined; + } + const direct = normalizeOptionalString(value.conversationId); + if (direct) { + return direct; + } + const receipt = isRecord(value.receipt) ? value.receipt : undefined; + if (!receipt) { + return undefined; + } + const candidates = [ + ...(Array.isArray(receipt.raw) ? receipt.raw : []), + ...(Array.isArray(receipt.parts) + ? receipt.parts.map((part) => (isRecord(part) ? part.raw : undefined)) + : []), + ]; + for (const candidate of candidates) { + if (!isRecord(candidate)) { + continue; + } + const conversationId = normalizeOptionalString(candidate.conversationId); + if (conversationId) { + return conversationId; + } + } + return undefined; +} + +/** Recover the actual Teams conversation resolved by a successful tool send. */ +export function extractMSTeamsToolSendResult( + result: unknown, + _send: ChannelToolSend, +): ChannelToolSend | null { + const details = isRecord(result) && isRecord(result.details) ? result.details : undefined; + const deliveryResult = details && isRecord(details.result) ? details.result : undefined; + const conversationId = extractMSTeamsResultConversationId(deliveryResult); + if (!conversationId) { + return null; + } + const normalizedConversationId = normalizeMSTeamsConversationId( + stripConversationPrefix(conversationId), + ); + return normalizedConversationId ? { to: `conversation:${normalizedConversationId}` } : null; +} + +export function msteamsContextTargetsMatch( + target: string, + context: { + currentChannelId?: string; + currentMessagingTarget?: string; + }, +): boolean { + const normalizedTarget = normalizeMSTeamsThreadingTarget(target); + if (!normalizedTarget) { + return false; + } + const currentChannel = normalizeMSTeamsThreadingTarget(context.currentChannelId); + if (currentChannel && currentChannel === normalizedTarget) { + return true; + } + const currentMessaging = normalizeMSTeamsThreadingTarget(context.currentMessagingTarget); + return Boolean(currentMessaging && currentMessaging === normalizedTarget); +} + +export function resolveMSTeamsAutoThreadId(params: { + cfg?: MSTeamsConfig; + to: string; + toolContext?: { + currentChannelId?: string; + currentMessagingTarget?: string; + currentGraphChannelId?: string; + currentThreadTs?: string; + replyToMode?: "off" | "first" | "all" | "batched"; + hasRepliedRef?: { value: boolean }; + }; +}): string | undefined { + const explicitThreadId = extractMSTeamsConversationMessageId(params.to); + if (explicitThreadId) { + return explicitThreadId; + } + const context = params.toolContext; + if (!context?.currentChannelId && !context?.currentMessagingTarget) { + return undefined; + } + if (!msteamsContextTargetsMatch(params.to, context)) { + return undefined; + } + const graphTarget = context.currentGraphChannelId ?? context.currentMessagingTarget; + const { team, channel } = graphTarget + ? parseMSTeamsTeamChannelInput(graphTarget) + : { team: undefined, channel: undefined }; + const routeConfig = resolveMSTeamsRouteConfig({ + cfg: params.cfg, + teamId: team, + conversationId: channel, + allowNameMatching: false, + }); + const { replyStyle } = resolveMSTeamsReplyPolicy({ + isDirectMessage: false, + globalConfig: params.cfg, + teamConfig: routeConfig.teamConfig, + channelConfig: routeConfig.channelConfig, + }); + if (replyStyle === "top-level") { + return undefined; + } + if (!context.currentThreadTs) { + return undefined; + } + if (context.replyToMode !== "all" && !isSingleUseReplyToMode(context.replyToMode ?? "off")) { + return undefined; + } + if (isSingleUseReplyToMode(context.replyToMode ?? "off") && context.hasRepliedRef?.value) { + return undefined; + } + return context.currentThreadTs; +} diff --git a/extensions/msteams/src/channel.actions.test.ts b/extensions/msteams/src/channel.actions.test.ts index 92d6960c16ca..5b209173c00e 100644 --- a/extensions/msteams/src/channel.actions.test.ts +++ b/extensions/msteams/src/channel.actions.test.ts @@ -123,6 +123,14 @@ function requireMSTeamsHandleAction() { return handleAction; } +function requireMSTeamsExtractToolSendResult() { + const extractToolSendResult = msteamsPlugin.actions?.extractToolSendResult; + if (!extractToolSendResult) { + throw new Error("msteams actions.extractToolSendResult unavailable"); + } + return extractToolSendResult; +} + async function runAction(params: { action: string; cfg?: Record; @@ -1354,6 +1362,7 @@ describe("msteamsPlugin.threading.buildToolContext", () => { To?: string; NativeChannelId?: string; ReplyToId?: string; + MessageThreadId?: string | number; }) { const build = msteamsPlugin.threading?.buildToolContext; if (!build) { @@ -1382,6 +1391,57 @@ describe("msteamsPlugin.threading.buildToolContext", () => { expect(result?.currentMessagingTarget).toBe("graph-team-1/19:channel-abc@thread.tacv2"); expect(result?.currentGraphChannelId).toBe("graph-team-1/19:channel-abc@thread.tacv2"); expect(result?.currentThreadTs).toBe("reply-1"); + expect(result?.replyToMode).toBe("all"); + }); + + it("prefers MessageThreadId (thread root) over ReplyToId (parent)", () => { + const result = callBuildToolContext({ + ChatType: "channel", + To: "conversation:19:channel-abc@thread.tacv2", + MessageThreadId: "thread-root", + ReplyToId: "nested-parent", + }); + expect(result?.currentThreadTs).toBe("thread-root"); + expect(result?.replyToMode).toBe("all"); + }); + + it("omits replyToMode when no thread context is present", () => { + const result = callBuildToolContext({ + ChatType: "direct", + To: "user:aad-user-1", + }); + expect(result?.currentThreadTs).toBeUndefined(); + expect(result?.replyToMode).toBeUndefined(); + }); + + it("does not stamp ReplyToId as ambient thread for DM turns", () => { + const result = callBuildToolContext({ + ChatType: "direct", + To: "user:aad-user-1", + ReplyToId: "quoted-parent", + }); + expect(result?.currentThreadTs).toBeUndefined(); + expect(result?.replyToMode).toBeUndefined(); + }); + + it("does not stamp ReplyToId as ambient thread for group turns", () => { + const result = callBuildToolContext({ + ChatType: "group", + To: "conversation:19:groupchat@thread.v2", + ReplyToId: "quoted-parent", + }); + expect(result?.currentThreadTs).toBeUndefined(); + expect(result?.replyToMode).toBeUndefined(); + }); + + it("stamps channel ReplyToId when MessageThreadId is absent", () => { + const result = callBuildToolContext({ + ChatType: "channel", + To: "conversation:19:channel-abc@thread.tacv2", + ReplyToId: "channel-parent", + }); + expect(result?.currentThreadTs).toBe("channel-parent"); + expect(result?.replyToMode).toBe("all"); }); it("falls back to To for DM turns (no NativeChannelId)", () => { @@ -1419,4 +1479,230 @@ describe("msteamsPlugin.threading.buildToolContext", () => { expect(result?.currentGraphChannelId).toBeUndefined(); }); }); + +describe("msteamsPlugin.actions.extractToolSendResult", () => { + it.each([ + { + name: "receipt raw", + result: { + details: { + result: { + receipt: { + raw: [{ conversationId: "19:channel@thread.tacv2" }], + }, + }, + }, + }, + }, + { + name: "receipt part raw", + result: { + details: { + result: { + receipt: { + parts: [{ raw: { conversationId: "19:channel@thread.tacv2" } }], + }, + }, + }, + }, + }, + { + name: "direct delivery result", + result: { + details: { + result: { + conversationId: "19:channel@thread.tacv2", + }, + }, + }, + }, + ])("canonicalizes a Graph target from $name", ({ result }) => { + expect( + requireMSTeamsExtractToolSendResult()({ + result, + send: { + to: "team-aad/19:channel@thread.tacv2", + threadId: "thread-root", + }, + }), + ).toEqual({ + to: "conversation:19:channel@thread.tacv2", + }); + }); + + it("canonicalizes user targets to the resolved conversation", () => { + expect( + requireMSTeamsExtractToolSendResult()({ + result: { + details: { + result: { + conversationId: "19:dm@thread.v2", + }, + }, + }, + send: { + to: "user:aad-user-1", + }, + }), + ).toEqual({ + to: "conversation:19:dm@thread.v2", + }); + }); + + it.each([ + undefined, + {}, + { details: {} }, + { details: { result: {} } }, + { details: { result: { receipt: { raw: [{}] } } } }, + ])("rejects a result without an authoritative conversation id", (result) => { + expect( + requireMSTeamsExtractToolSendResult()({ + result, + send: { + to: "team-aad/19:channel@thread.tacv2", + threadId: "thread-root", + }, + }), + ).toBeNull(); + }); +}); + +describe("msteamsPlugin.threading.resolveAutoThreadId", () => { + function resolveAutoThreadId(params: { + cfg?: OpenClawConfig; + to?: string; + currentGraphChannelId?: string; + }) { + const resolve = msteamsPlugin.threading?.resolveAutoThreadId; + if (!resolve) { + throw new Error("msteams threading.resolveAutoThreadId unavailable"); + } + return resolve({ + cfg: params.cfg ?? ({} as OpenClawConfig), + to: params.to ?? "conversation:19:channel@thread.tacv2", + toolContext: { + currentChannelId: "conversation:19:channel@thread.tacv2", + currentMessagingTarget: params.currentGraphChannelId, + currentGraphChannelId: params.currentGraphChannelId, + currentThreadTs: "thread-root", + replyToMode: "all", + }, + }); + } + + it("returns ambient thread root for same conversation target", () => { + expect(resolveAutoThreadId({})).toBe("thread-root"); + }); + + it("returns undefined for a global top-level reply style", () => { + expect( + resolveAutoThreadId({ + cfg: { + channels: { + msteams: { + replyStyle: "top-level", + }, + }, + } as OpenClawConfig, + }), + ).toBeUndefined(); + }); + + it("returns undefined when requireMention defaults the reply style to top-level", () => { + expect( + resolveAutoThreadId({ + cfg: { + channels: { + msteams: { + requireMention: false, + }, + }, + } as OpenClawConfig, + }), + ).toBeUndefined(); + }); + + it("honors team and channel reply style overrides", () => { + const cfg = { + channels: { + msteams: { + replyStyle: "thread", + teams: { + "team-1": { + replyStyle: "top-level", + channels: { + "19:channel@thread.tacv2": { + replyStyle: "thread", + }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + + expect( + resolveAutoThreadId({ + cfg, + currentGraphChannelId: "team-1/19:other@thread.tacv2", + }), + ).toBeUndefined(); + expect( + resolveAutoThreadId({ + cfg, + currentGraphChannelId: "team-1/19:channel@thread.tacv2", + }), + ).toBe("thread-root"); + }); + + it("preserves an explicit thread target under top-level reply style", () => { + expect( + resolveAutoThreadId({ + cfg: { + channels: { + msteams: { + replyStyle: "top-level", + }, + }, + } as OpenClawConfig, + to: "conversation:19:channel@thread.tacv2;messageid=explicit-root", + }), + ).toBe("explicit-root"); + }); + + it("returns undefined for a different conversation", () => { + const resolve = msteamsPlugin.threading?.resolveAutoThreadId; + if (!resolve) { + throw new Error("msteams threading.resolveAutoThreadId unavailable"); + } + expect( + resolve({ + cfg: {} as OpenClawConfig, + to: "conversation:19:other@thread.tacv2", + toolContext: { + currentChannelId: "conversation:19:channel@thread.tacv2", + currentThreadTs: "thread-root", + replyToMode: "all", + }, + }), + ).toBeUndefined(); + }); + + it("returns undefined for DM tool context without ambient thread", () => { + const resolve = msteamsPlugin.threading?.resolveAutoThreadId; + if (!resolve) { + throw new Error("msteams threading.resolveAutoThreadId unavailable"); + } + expect( + resolve({ + cfg: {} as OpenClawConfig, + to: "user:aad-user-1", + toolContext: { + currentChannelId: "user:aad-user-1", + }, + }), + ).toBeUndefined(); + }); +}); /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/extensions/msteams/src/channel.message-adapter.test.ts b/extensions/msteams/src/channel.message-adapter.test.ts index 99b10e4cba76..e11d4950eb65 100644 --- a/extensions/msteams/src/channel.message-adapter.test.ts +++ b/extensions/msteams/src/channel.message-adapter.test.ts @@ -225,4 +225,39 @@ describe("msteams channel message adapter", () => { }, }); }); + + it("exposes the resolved conversation for message-tool route reconciliation", async () => { + const adapter = requireMSTeamsMessageAdapter(); + const sendText = requireTextSender(adapter); + const extractToolSendResult = msteamsPlugin.actions?.extractToolSendResult; + if (!extractToolSendResult) { + throw new Error("Expected msteams tool send result extractor"); + } + + const delivery = await sendText({ + cfg, + to: "team-aad/19:channel@thread.tacv2", + text: "threaded", + threadId: "thread-root", + accountId: "default", + }); + + expect( + extractToolSendResult({ + result: { + details: { + result: { + receipt: delivery.receipt, + }, + }, + }, + send: { + to: "team-aad/19:channel@thread.tacv2", + threadId: "thread-root", + }, + }), + ).toEqual({ + to: "conversation:conv-1", + }); + }); }); diff --git a/extensions/msteams/src/channel.ts b/extensions/msteams/src/channel.ts index e3584191cf4f..a47172f5c709 100644 --- a/extensions/msteams/src/channel.ts +++ b/extensions/msteams/src/channel.ts @@ -40,6 +40,11 @@ import { DEFAULT_ACCOUNT_ID, PAIRING_APPROVED_MESSAGE, } from "../runtime-api.js"; +import { + extractMSTeamsToolSendResult, + msteamsContextTargetsMatch, + resolveMSTeamsAutoThreadId, +} from "./action-threading.js"; import { msTeamsApprovalAuth } from "./approval-auth.js"; import { msteamsConfigAdapter, @@ -720,6 +725,7 @@ export const msteamsPlugin: ChannelPlugin extractMSTeamsToolSendResult(result, send), requiresTrustedRequesterSender: ({ action, toolContext }) => normalizeOptionalString(toolContext?.currentChannelProvider)?.toLowerCase() === "msteams" && MSTEAMS_GROUP_MANAGEMENT_ACTIONS.has(action), @@ -1287,9 +1293,20 @@ export const msteamsPlugin: ChannelPlugin + msteamsContextTargetsMatch(target, toolContext), buildToolContext: ({ context, hasRepliedRef }) => { const nativeChannelId = context.NativeChannelId?.trim(); const hasChannelRoute = Boolean(nativeChannelId && nativeChannelId.includes("/")); + const isChannel = context.ChatType === "channel"; + const messageThreadId = + context.MessageThreadId != null + ? normalizeOptionalString(String(context.MessageThreadId)) + : undefined; + // Prefer MessageThreadId (root). ReplyToId fallback is channel-only — DM/group + // quote replies must not inherit ambient thread metadata for dedupe. + const currentThreadTs = + messageThreadId ?? (isChannel ? normalizeOptionalString(context.ReplyToId) : undefined); return { currentChannelId: normalizeOptionalString(context.To), currentChatType: @@ -1300,10 +1317,17 @@ export const msteamsPlugin: ChannelPlugin + resolveMSTeamsAutoThreadId({ + cfg: cfg.channels?.msteams, + to, + toolContext, + }), }, outbound: msteamsChannelOutbound, }); diff --git a/extensions/msteams/src/monitor-handler/inbound-dispatch.ts b/extensions/msteams/src/monitor-handler/inbound-dispatch.ts index 26e5c5d9df97..243aa873389d 100644 --- a/extensions/msteams/src/monitor-handler/inbound-dispatch.ts +++ b/extensions/msteams/src/monitor-handler/inbound-dispatch.ts @@ -182,6 +182,12 @@ export async function dispatchMSTeamsInboundTurn(params: { }, reply: { to: teamsTo, + // A user target is a mutable lookup alias; the conversation id is the + // authoritative route for message-tool reply suppression. + originatingTo: `conversation:${conversationId}`, + // Channel-only thread root (facts.threadId) so messaging-tool evidence matches + // session/origin thread ids; keep replyToId as parent. Undefined for DM/group. + messageThreadId: facts.threadId ?? undefined, replyToId: activity.replyToId ?? undefined, nativeChannelId, }, diff --git a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts index 4edf5853a04b..576fbd44f825 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts @@ -1031,6 +1031,8 @@ describe("msteams monitor handler authz", () => { }), ); const ctx = recordFromMockCall(firstSettledDispatch().ctxPayload); + expect(ctx.To).toBe("user:user-aad"); + expect(ctx.OriginatingTo).toBe("conversation:19:dm@thread.v2"); expect(ctx.ReplyToId).toBe("message-1"); expect(ctx.ReplyToBody).toBe("complete quoted message"); expect(ctx.ReplyToSender).toBe("Bot"); diff --git a/extensions/msteams/src/outbound.test.ts b/extensions/msteams/src/outbound.test.ts index 0978f7169ea2..f7765ac69e40 100644 --- a/extensions/msteams/src/outbound.test.ts +++ b/extensions/msteams/src/outbound.test.ts @@ -169,6 +169,66 @@ describe("msteamsOutbound cfg threading", () => { }); }); + it("forwards resolved channel thread ids through the Teams target", async () => { + await requireSendText()({ + cfg, + to: "conversation:19:channel@thread.tacv2", + text: "threaded", + threadId: "thread-root-2", + }); + + expect(mocks.sendMessageMSTeams).toHaveBeenCalledWith({ + cfg, + to: "conversation:19:channel@thread.tacv2;messageid=thread-root-2", + text: "threaded", + }); + }); + + it("preserves explicit Teams thread targets", async () => { + await requireSendText()({ + cfg, + to: "conversation:19:channel@thread.tacv2;messageid=explicit-root", + text: "threaded", + threadId: "ambient-root", + }); + + expect(mocks.sendMessageMSTeams).toHaveBeenCalledWith({ + cfg, + to: "conversation:19:channel@thread.tacv2;messageid=explicit-root", + text: "threaded", + }); + }); + + it("forwards thread ids through Graph team/channel targets", async () => { + await requireSendText()({ + cfg, + to: "graph-team/19:channel@thread.tacv2", + text: "threaded", + threadId: "thread-root-3", + }); + + expect(mocks.sendMessageMSTeams).toHaveBeenCalledWith({ + cfg, + to: "graph-team/19:channel@thread.tacv2;messageid=thread-root-3", + text: "threaded", + }); + }); + + it("does not append channel thread ids to direct-message targets", async () => { + await requireSendText()({ + cfg, + to: "user:aad-user-1", + text: "direct", + threadId: "quoted-parent", + }); + + expect(mocks.sendMessageMSTeams).toHaveBeenCalledWith({ + cfg, + to: "user:aad-user-1", + text: "direct", + }); + }); + it("passes resolved cfg and media roots for media sends", async () => { const cfgValue = { channels: { diff --git a/extensions/msteams/src/outbound.ts b/extensions/msteams/src/outbound.ts index c1bdb5ec16a4..163226dab3a4 100644 --- a/extensions/msteams/src/outbound.ts +++ b/extensions/msteams/src/outbound.ts @@ -44,6 +44,22 @@ type MSTeamsMediaSendFn = ( opts?: MSTeamsMediaSendOptions, ) => Promise; +function resolveMSTeamsThreadTarget(to: string, threadId?: string | number | null) { + const normalizedThreadId = threadId == null ? "" : String(threadId).trim(); + const graphChannelId = to.includes("/") ? to.slice(to.indexOf("/") + 1) : ""; + const isConversationTarget = + to.startsWith("conversation:") || + to.startsWith("19:") || + graphChannelId.startsWith("19:") || + graphChannelId.includes("@thread"); + // Keep the resolved root on the target so proactive lookup and Connector + // delivery use this turn's thread, not the latest stored conversation root. + if (!normalizedThreadId || /(?:^|;)messageid=/iu.test(to) || !isConversationTarget) { + return to; + } + return `${to};messageid=${normalizedThreadId}`; +} + function resolveMSTeamsTextSend(params: { cfg: MSTeamsSendConfig; deps?: OutboundSendDeps; @@ -119,7 +135,9 @@ export const msteamsOutbound: ChannelOutboundAdapter = { payload, deps, onDeliveryResult, + threadId, }) => { + const deliveryTarget = resolveMSTeamsThreadTarget(to, threadId); const msteamsData = asOptionalRecord(payload.channelData?.msteams); const presentationCard = msteamsData?.presentationCard; if ( @@ -129,7 +147,7 @@ export const msteamsOutbound: ChannelOutboundAdapter = { ) { const result = await sendAdaptiveCardMSTeams({ cfg, - to, + to: deliveryTarget, card: presentationCard as Record, }); return attachChannelToResult("msteams", result); @@ -149,7 +167,11 @@ export const msteamsOutbound: ChannelOutboundAdapter = { await onDeliveryResult?.(attachChannelToResult("msteams", deliveryResult)); }, send: async ({ text: textLocal, mediaUrl: mediaUrlLocal }) => - await send(to, textLocal, { mediaUrl: mediaUrlLocal, mediaLocalRoots, mediaReadFile }), + await send(deliveryTarget, textLocal, { + mediaUrl: mediaUrlLocal, + mediaLocalRoots, + mediaReadFile, + }), }); if (result) { return attachChannelToResult("msteams", result); @@ -166,7 +188,7 @@ export const msteamsOutbound: ChannelOutboundAdapter = { ); let result: Awaited>; for (const chunk of chunks) { - result = await send(to, chunk); + result = await send(deliveryTarget, chunk); await onDeliveryResult?.(attachChannelToResult("msteams", result)); } return attachChannelToResult("msteams", result!); @@ -175,13 +197,26 @@ export const msteamsOutbound: ChannelOutboundAdapter = { }, ...createAttachedChannelResultAdapter({ channel: "msteams", - sendText: async ({ cfg, to, text, deps }) => { + sendText: async ({ cfg, to, text, deps, threadId }) => { const send = resolveMSTeamsTextSend({ cfg, deps }); - return await send(to, text); + return await send(resolveMSTeamsThreadTarget(to, threadId), text); }, - sendMedia: async ({ cfg, to, text, mediaUrl, mediaLocalRoots, mediaReadFile, deps }) => { + sendMedia: async ({ + cfg, + to, + text, + mediaUrl, + mediaLocalRoots, + mediaReadFile, + deps, + threadId, + }) => { const send = resolveMSTeamsMediaSend({ cfg, deps }); - return await send(to, text, { mediaUrl, mediaLocalRoots, mediaReadFile }); + return await send(resolveMSTeamsThreadTarget(to, threadId), text, { + mediaUrl, + mediaLocalRoots, + mediaReadFile, + }); }, sendPoll: async ({ cfg, to, poll }) => { const maxSelections = poll.maxSelections ?? 1; diff --git a/extensions/msteams/src/qa/adapter.runtime.test.ts b/extensions/msteams/src/qa/adapter.runtime.test.ts index 3610ab593b59..a2ad422931cd 100644 --- a/extensions/msteams/src/qa/adapter.runtime.test.ts +++ b/extensions/msteams/src/qa/adapter.runtime.test.ts @@ -68,6 +68,10 @@ describe("Microsoft Teams QA transport adapter", () => { const config = adapter.createGatewayConfig({ baseUrl: "http://127.0.0.1" }); const webhookPort = config.channels?.msteams?.webhook?.port; expect(webhookPort).toEqual(expect.any(Number)); + expect(config.channels?.msteams).toMatchObject({ + dmPolicy: "allowlist", + allowFrom: ["00000000-0000-4000-8000-000000000002"], + }); let inboundActivity: Record | undefined; const webhook = createServer((request, response) => { void (async () => { @@ -89,6 +93,7 @@ describe("Microsoft Teams QA transport adapter", () => { senderName: "Driver", text: "@openclaw qa ingress", threadId: "thread-root", + replyToId: "quoted-parent", }); expect(inboundActivity).toMatchObject({ text: "openclaw qa ingress", @@ -100,6 +105,11 @@ describe("Microsoft Teams QA transport adapter", () => { }, ], serviceUrl: "https://smba.trafficmanager.net/qa", + replyToId: "quoted-parent", + from: { + id: "qa-msteams-driver", + aadObjectId: "00000000-0000-4000-8000-000000000002", + }, conversation: { id: "19:qa-primary@thread.tacv2;messageid=thread-root", conversationType: "channel", diff --git a/extensions/msteams/src/qa/adapter.runtime.ts b/extensions/msteams/src/qa/adapter.runtime.ts index 6ed826453533..bdcbe45a0031 100644 --- a/extensions/msteams/src/qa/adapter.runtime.ts +++ b/extensions/msteams/src/qa/adapter.runtime.ts @@ -22,6 +22,7 @@ const SERVICE_URL = "https://smba.trafficmanager.net/qa"; const APP_ID = "qa-msteams-app"; const TENANT_ID = "qa-msteams-tenant"; const DRIVER_ID = "qa-msteams-driver"; +const DRIVER_AAD_OBJECT_ID = "00000000-0000-4000-8000-000000000002"; const TEAM_ID = "qa-msteams-team"; const TEAM_AAD_GROUP_ID = "00000000-0000-4000-8000-000000000001"; const DEFAULT_ACCOUNT_ID = "default"; @@ -170,7 +171,7 @@ export async function createMSTeamsQaTransportAdapter( serviceUrl: SERVICE_URL, from: { id: DRIVER_ID, - aadObjectId: DRIVER_ID, + aadObjectId: DRIVER_AAD_OBJECT_ID, name: input.senderName ?? "Teams QA Driver", }, recipient: { id: APP_ID, name: "OpenClaw QA" }, @@ -184,6 +185,7 @@ export async function createMSTeamsQaTransportAdapter( : "channel", tenantId: TENANT_ID, }, + ...(input.replyToId ? { replyToId: input.replyToId } : {}), channelData: { tenant: { id: TENANT_ID }, team: { id: TEAM_ID, aadGroupId: TEAM_AAD_GROUP_ID }, @@ -236,6 +238,8 @@ export async function createMSTeamsQaTransportAdapter( appId: APP_ID, appPassword: "private-qa-secret", tenantId: TENANT_ID, + dmPolicy: "allowlist", + allowFrom: [DRIVER_AAD_OBJECT_ID], groupPolicy: "open", requireMention: requireGroupMention, replyStyle: "thread", diff --git a/extensions/msteams/src/send-context.test.ts b/extensions/msteams/src/send-context.test.ts index 0b9dd9542f05..727b28efc9b8 100644 --- a/extensions/msteams/src/send-context.test.ts +++ b/extensions/msteams/src/send-context.test.ts @@ -128,6 +128,68 @@ describe("resolveMSTeamsSendContext", () => { }); }); + it("looks up the base conversation and applies an explicit thread root", async () => { + sendContextMockState.store.get.mockResolvedValue( + channelRef({ + serviceUrl: "https://smba.trafficmanager.net/amer/", + threadId: "stored-root", + }), + ); + + await expect( + resolveMSTeamsSendContext({ + cfg: { + channels: { + msteams: { + enabled: true, + appId: "app-id", + appPassword: "app-password", + tenantId: "tenant-id", + replyStyle: "top-level", + }, + }, + } as OpenClawConfig, + to: "conversation:19:channel@thread.tacv2;messageid=explicit-root", + }), + ).resolves.toMatchObject({ + conversationId: "19:channel@thread.tacv2", + ref: { threadId: "explicit-root" }, + replyStyle: "thread", + }); + expect(sendContextMockState.store.get).toHaveBeenCalledWith("19:channel@thread.tacv2"); + }); + + it("resolves Graph team/channel targets through the stored channel conversation", async () => { + sendContextMockState.store.get.mockResolvedValue( + channelRef({ + serviceUrl: "https://smba.trafficmanager.net/amer/", + threadId: "stored-root", + }), + ); + + await expect( + resolveMSTeamsSendContext({ + cfg: { + channels: { + msteams: { + enabled: true, + appId: "app-id", + appPassword: "app-password", + tenantId: "tenant-id", + replyStyle: "top-level", + }, + }, + } as OpenClawConfig, + to: "graph-team/19:channel@thread.tacv2;messageid=graph-root", + }), + ).resolves.toMatchObject({ + conversationId: "19:channel@thread.tacv2", + ref: { threadId: "graph-root" }, + replyStyle: "thread", + }); + expect(sendContextMockState.store.get).toHaveBeenCalledWith("19:channel@thread.tacv2"); + }); + it("removes stored conversation references with blocked serviceUrl hosts", async () => { sendContextMockState.store.get.mockResolvedValue( channelRef({ diff --git a/extensions/msteams/src/send-context.ts b/extensions/msteams/src/send-context.ts index 0c973c0eef25..1f60a705e402 100644 --- a/extensions/msteams/src/send-context.ts +++ b/extensions/msteams/src/send-context.ts @@ -24,6 +24,7 @@ import type { StoredConversationReference, } from "./conversation-store.js"; import { formatUnknownError } from "./errors.js"; +import { extractMSTeamsConversationMessageId, normalizeMSTeamsConversationId } from "./inbound.js"; import { resolveMSTeamsReplyPolicy, resolveMSTeamsRouteConfig } from "./policy.js"; import { getMSTeamsRuntime } from "./runtime.js"; import type { MSTeamsApp } from "./sdk.js"; @@ -82,12 +83,14 @@ function resolveMSTeamsProactiveReplyStyle(params: { * Parse the target value into a conversation reference lookup key. * Supported formats: * - conversation:19:abc@thread.tacv2 → lookup by conversation ID + * - conversation:19:abc@thread.tacv2;messageid=root → lookup base ID, use root * - user:aad-object-id → lookup by user AAD object ID * - 19:abc@thread.tacv2 → direct conversation ID */ function parseRecipient(to: string): { type: "conversation" | "user"; id: string; + threadId?: string; } { const trimmed = to.trim(); const finalize = (type: "conversation" | "user", id: string) => { @@ -95,6 +98,21 @@ function parseRecipient(to: string): { if (!normalized) { throw new Error(`Invalid target value: missing ${type} id`); } + if (type === "conversation") { + const threadId = extractMSTeamsConversationMessageId(normalized); + const normalizedConversationId = normalizeMSTeamsConversationId(normalized); + const slashIndex = normalizedConversationId.indexOf("/"); + const graphChannelId = + slashIndex > 0 ? normalizedConversationId.slice(slashIndex + 1) : undefined; + return { + type, + id: + graphChannelId && (graphChannelId.startsWith("19:") || graphChannelId.includes("@thread")) + ? graphChannelId + : normalizedConversationId, + ...(threadId ? { threadId } : {}), + }; + } return { type, id: normalized }; }; if (trimmed.startsWith("conversation:")) { @@ -165,7 +183,8 @@ export async function resolveMSTeamsSendContext(params: { ); } - const { conversationId, ref } = found; + const conversationId = found.conversationId; + const ref = recipient.threadId ? { ...found.ref, threadId: recipient.threadId } : found.ref; const core = getMSTeamsRuntime(); const log = core.logging.getChildLogger({ name: "msteams:send" }); @@ -228,12 +247,18 @@ export async function resolveMSTeamsSendContext(params: { // groupChat, or unknown defaults to groupChat behavior conversationType = "groupChat"; } - const replyStyle = resolveMSTeamsProactiveReplyStyle({ - cfg: msteamsCfg, - conversationId, - ref: safeRef, - conversationType, - }); + // An explicit messageid is a caller-owned destination. Ambient and stored + // roots still obey route policy, but explicit channel roots must not be + // flattened by a top-level default. + const replyStyle = + recipient.threadId && conversationType === "channel" + ? "thread" + : resolveMSTeamsProactiveReplyStyle({ + cfg: msteamsCfg, + conversationId, + ref: safeRef, + conversationType, + }); // Get SharePoint site ID from config (required for file uploads in group chats/channels) const sharePointSiteId = msteamsCfg.sharePointSiteId; diff --git a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts index de23202ad2b9..c292d0d33f55 100644 --- a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts +++ b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts @@ -192,6 +192,7 @@ export const QA_TOOL_PROGRESS_PROMPT_RE = /tool progress qa check/i; export const QA_TOOL_LOOP_GLOBAL_BREAKER_PROMPT_RE = /global tool loop breaker qa check/i; export const QA_PROVIDER_HTTP_503_AFTER_TOOL_PROMPT_RE = /provider http 503 after tool qa check/i; export const QA_GROUP_VISIBLE_REPLY_TOOL_PROMPT_RE = /qa group visible reply tool check/i; +export const QA_MSTEAMS_THREAD_DEDUPE_PROMPT_RE = /qa msteams thread message-tool final dedupe/i; export const QA_A2A_MESSAGE_TOOL_MIRROR_PROMPT_RE = /qa a2a message-tool mirror check/i; export const QA_GROUP_MESSAGE_UNAVAILABLE_FALLBACK_PROMPT_RE = /qa group message unavailable fallback check/i; diff --git a/extensions/qa-lab/src/providers/mock-openai/server.test.ts b/extensions/qa-lab/src/providers/mock-openai/server.test.ts index 2ba647d3f38a..4f50109d2ea2 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.test.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.test.ts @@ -537,6 +537,44 @@ describe("qa mock openai server", () => { }); }); + it("returns the same Teams final after the message-tool send", async () => { + const server = await startMockServer(); + const prompt = [ + "qa msteams thread message-tool final dedupe.", + "msteams message target: `conversation:19:other@thread.tacv2;messageid=other-root`.", + "exact marker: `QA-MSTEAMS-THREAD-DEDUPE-OK`", + ].join(" "); + + const initialBody = await expectResponsesJson(server, { + stream: false, + model: "gpt-5.6-luna", + tools: [MESSAGE_TOOL], + input: [makeUserInput(prompt)], + }); + const toolCall = outputToolCall(initialBody, "message"); + expect(outputToolArgsFromItem(toolCall)).toEqual({ + action: "send", + message: "QA-MSTEAMS-THREAD-DEDUPE-OK", + target: "conversation:19:other@thread.tacv2;messageid=other-root", + }); + + const finalBody = await expectResponsesJson(server, { + stream: false, + model: "gpt-5.6-luna", + tools: [MESSAGE_TOOL], + input: [ + makeUserInput(prompt), + { + type: "function_call_output", + call_id: outputToolCallId(toolCall, "call_msteams_thread_dedupe"), + output: JSON.stringify({ ok: true }), + }, + ], + }); + expect(outputText(finalBody)).toBe("QA-MSTEAMS-THREAD-DEDUPE-OK"); + expect(outputItems(finalBody).some((item) => item.type === "function_call")).toBe(false); + }); + it("keeps the retry-failure stranded-final fixture as text without a message tool call", async () => { const server = await startMockServer(); diff --git a/extensions/qa-lab/src/providers/mock-openai/server.ts b/extensions/qa-lab/src/providers/mock-openai/server.ts index 1b37346e24e4..e4dac7ddadb2 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.ts @@ -33,6 +33,7 @@ import { QA_TOOL_LOOP_GLOBAL_BREAKER_PROMPT_RE, QA_PROVIDER_HTTP_503_AFTER_TOOL_PROMPT_RE, QA_GROUP_VISIBLE_REPLY_TOOL_PROMPT_RE, + QA_MSTEAMS_THREAD_DEDUPE_PROMPT_RE, QA_A2A_MESSAGE_TOOL_MIRROR_PROMPT_RE, QA_GROUP_MESSAGE_UNAVAILABLE_FALLBACK_PROMPT_RE, QA_STRANDED_FINAL_RECOVERY_PROMPT_RE, @@ -1021,6 +1022,18 @@ async function buildResponsesPayload( } return buildAssistantEvents(""); } + if (QA_MSTEAMS_THREAD_DEDUPE_PROMPT_RE.test(allInputText)) { + const marker = exactMarkerDirective ?? exactReplyDirective ?? "QA-MSTEAMS-THREAD-DEDUPE-OK"; + const target = /msteams message target:\s*`([^`]+)`/iu.exec(prompt)?.[1]?.trim(); + if (!toolOutput && hasDeclaredTool(body, "message")) { + return buildToolCallEventsWithArgs("message", { + action: "send", + message: marker, + ...(target ? { target } : {}), + }); + } + return buildAssistantEvents(marker); + } if (QA_GROUP_MESSAGE_UNAVAILABLE_FALLBACK_PROMPT_RE.test(allInputText)) { return buildAssistantEvents( exactMarkerDirective ?? exactReplyDirective ?? "QA-GROUP-FALLBACK-OK", diff --git a/extensions/qa-lab/src/scenario-catalog-channels.test.ts b/extensions/qa-lab/src/scenario-catalog-channels.test.ts index 570852f39fad..809885974f56 100644 --- a/extensions/qa-lab/src/scenario-catalog-channels.test.ts +++ b/extensions/qa-lab/src/scenario-catalog-channels.test.ts @@ -67,6 +67,26 @@ describe("qa scenario catalog channel contracts", () => { } }); + it("keeps the Teams final-dedupe proof on the real Gateway transport", () => { + const scenario = requireFlowScenario( + readQaScenarioById("msteams-thread-message-tool-final-dedupe"), + ); + const flow = JSON.stringify(scenario.execution.flow); + + expect(scenario.execution.channel).toBe("msteams"); + expect(scenario.execution.suiteIsolation).toBe("isolated"); + expect(scenario.gatewayConfigPatch).toMatchObject({ + messages: { groupChat: { visibleReplies: "automatic" } }, + tools: { alsoAllow: ["message"] }, + agents: { entries: { qa: { tools: { alsoAllow: ["message"] } } } }, + }); + expect(flow).toContain("QA-MSTEAMS-SAME-OK"); + expect(flow).toContain("QA-MSTEAMS-OTHER-THREAD-OK"); + expect(flow).toContain("QA-MSTEAMS-OTHER-CONVERSATION-OK"); + expect(flow).toContain("QA-MSTEAMS-DM-OK"); + expect(flow).toContain("QA-MSTEAMS-GROUP-OK"); + }); + it("isolates scenarios that own asynchronous transport state", () => { const channelBaseline = requireFlowScenario(readQaScenarioById("channel-chat-baseline")); const subagentFanout = requireFlowScenario(readQaScenarioById("subagent-fanout-synthesis")); diff --git a/qa/scenarios/channels/msteams-thread-message-tool-final-dedupe.yaml b/qa/scenarios/channels/msteams-thread-message-tool-final-dedupe.yaml new file mode 100644 index 000000000000..8b673867f088 --- /dev/null +++ b/qa/scenarios/channels/msteams-thread-message-tool-final-dedupe.yaml @@ -0,0 +1,243 @@ +title: Microsoft Teams thread message-tool final dedupe + +scenario: + id: msteams-thread-message-tool-final-dedupe + surface: channels + coverage: + primary: + - channels.message-final-reply + secondary: + - channels.native-threads + - channels.outbound-message + objective: Verify the real Microsoft Teams Gateway path suppresses an automatic final only when message(action=send) already delivered the same text to the same conversation thread. + gatewayConfigPatch: + messages: + groupChat: + visibleReplies: automatic + tools: + alsoAllow: + - message + agents: + entries: + qa: + tools: + alsoAllow: + - message + successCriteria: + - The real bundled Microsoft Teams plugin receives Bot Framework channel, group, and direct-message activities. + - The mock provider calls message(action=send), then returns the identical marker as its automatic final. + - Same-thread, direct-message, and group-chat turns deliver the marker exactly once. + - Different-thread and different-conversation tool sends retain the automatic final on the source route. + regressionRefs: + - https://github.com/openclaw/openclaw/issues/116397 + docsRefs: + - docs/channels/msteams.md + - docs/concepts/qa-e2e-automation.md + codeRefs: + - extensions/msteams/src/monitor-handler/inbound-dispatch.ts + - extensions/msteams/src/channel.ts + - extensions/msteams/src/action-threading.ts + - extensions/msteams/src/outbound.ts + - extensions/msteams/src/send-context.ts + - src/auto-reply/reply/reply-payloads-dedupe.ts + execution: + kind: flow + channel: msteams + suiteIsolation: isolated + isolationReason: Seeds multiple native Teams conversation references and thread roots. + summary: Run message-tool and automatic-final delivery through a real Gateway, the Teams plugin, a loopback Bot Framework connector, and the mock provider. + config: + requiredProviderMode: mock-openai + duplicateWindowMs: 8000 + promptSnippet: qa msteams thread message-tool final dedupe + +flow: + steps: + - name: suppresses only the same Teams route + actions: + - assert: + expr: "env.providerMode === config.requiredProviderMode" + message: this seeded scenario is mock-openai only + - call: waitForGatewayHealthy + args: + - ref: env + - 60000 + - call: waitForTransportReady + args: + - ref: env + - 60000 + - call: reset + + - sendInbound: + conversation: { id: dedupe-same, kind: channel, title: Dedupe Same Thread } + senderId: driver + senderName: Teams QA Driver + text: "@openclaw Reply exactly: QA-MSTEAMS-SAME-ROOT" + saveAs: sameRoot + - waitForOutbound: + conversation: { id: dedupe-same, kind: channel } + textIncludes: QA-MSTEAMS-SAME-ROOT + timeoutMs: + expr: liveTurnTimeoutMs(env, 90000) + - set: sameStart + value: + expr: state.getSnapshot().messages.length + - set: requestCursorBeforeSame + value: + expr: "(await fetchJson(`${env.mock.baseUrl}/debug/request-cursor`)).cursor" + - sendInbound: + conversation: { id: dedupe-same, kind: channel, title: Dedupe Same Thread } + senderId: driver + senderName: Teams QA Driver + threadId: + ref: sameRoot.id + text: "@openclaw qa msteams thread message-tool final dedupe. exact marker: `QA-MSTEAMS-SAME-OK`" + - call: waitForCondition + args: + - lambda: + async: true + params: [] + expr: "(await fetchJson(`${env.mock.baseUrl}/debug/requests?after=${requestCursorBeforeSame}`)).some((request) => request.plannedToolName === 'message' && request.plannedToolArgs?.message === 'QA-MSTEAMS-SAME-OK')" + - expr: liveTurnTimeoutMs(env, 90000) + - 500 + - call: sleep + args: + - expr: config.duplicateWindowMs + - set: sameOutbound + value: + expr: "state.getSnapshot().messages.slice(sameStart).filter((message) => message.direction === 'outbound' && message.conversation.id === 'dedupe-same' && String(message.text ?? '').includes('QA-MSTEAMS-SAME-OK'))" + - assert: + expr: "sameOutbound.length === 1 && sameOutbound[0]?.threadId === sameRoot.id" + message: + expr: "`expected one same-thread marker on ${sameRoot.id}; saw ${JSON.stringify(sameOutbound)}; transcript=${formatTransportTranscript(state, { conversationId: 'dedupe-same' })}`" + + - sendInbound: + conversation: { id: dedupe-other-thread, kind: channel, title: Dedupe Other Thread } + senderId: driver + senderName: Teams QA Driver + text: "@openclaw Reply exactly: QA-MSTEAMS-THREAD-A-ROOT" + saveAs: threadARoot + - waitForOutbound: + conversation: { id: dedupe-other-thread, kind: channel } + textIncludes: QA-MSTEAMS-THREAD-A-ROOT + timeoutMs: + expr: liveTurnTimeoutMs(env, 90000) + - sendInbound: + conversation: { id: dedupe-other-thread, kind: channel, title: Dedupe Other Thread } + senderId: driver + senderName: Teams QA Driver + text: "@openclaw Reply exactly: QA-MSTEAMS-THREAD-B-ROOT" + saveAs: threadBRoot + - waitForOutbound: + conversation: { id: dedupe-other-thread, kind: channel } + textIncludes: QA-MSTEAMS-THREAD-B-ROOT + timeoutMs: + expr: liveTurnTimeoutMs(env, 90000) + - set: otherThreadStart + value: + expr: state.getSnapshot().messages.length + - sendInbound: + conversation: { id: dedupe-other-thread, kind: channel, title: Dedupe Other Thread } + senderId: driver + senderName: Teams QA Driver + threadId: + ref: threadARoot.id + text: + expr: "`@openclaw qa msteams thread message-tool final dedupe. msteams message target: \\`conversation:19:dedupe-other-thread@thread.tacv2;messageid=${threadBRoot.id}\\`. exact marker: \\`QA-MSTEAMS-OTHER-THREAD-OK\\``" + - call: sleep + args: + - expr: config.duplicateWindowMs + - set: otherThreadOutbound + value: + expr: "state.getSnapshot().messages.slice(otherThreadStart).filter((message) => message.direction === 'outbound' && message.conversation.id === 'dedupe-other-thread' && String(message.text ?? '').includes('QA-MSTEAMS-OTHER-THREAD-OK'))" + - assert: + expr: "otherThreadOutbound.length === 2 && otherThreadOutbound.some((message) => message.threadId === threadARoot.id) && otherThreadOutbound.some((message) => message.threadId === threadBRoot.id)" + message: + expr: "`expected one marker in each Teams thread; saw ${JSON.stringify(otherThreadOutbound)}; transcript=${formatTransportTranscript(state, { conversationId: 'dedupe-other-thread' })}`" + + - sendInbound: + conversation: + { id: dedupe-target-conversation, kind: channel, title: Dedupe Target Conversation } + senderId: driver + senderName: Teams QA Driver + text: "@openclaw Reply exactly: QA-MSTEAMS-TARGET-CONVERSATION-ROOT" + saveAs: targetConversationRoot + - waitForOutbound: + conversation: { id: dedupe-target-conversation, kind: channel } + textIncludes: QA-MSTEAMS-TARGET-CONVERSATION-ROOT + timeoutMs: + expr: liveTurnTimeoutMs(env, 90000) + - sendInbound: + conversation: + { id: dedupe-source-conversation, kind: channel, title: Dedupe Source Conversation } + senderId: driver + senderName: Teams QA Driver + text: "@openclaw Reply exactly: QA-MSTEAMS-SOURCE-CONVERSATION-ROOT" + saveAs: sourceConversationRoot + - waitForOutbound: + conversation: { id: dedupe-source-conversation, kind: channel } + textIncludes: QA-MSTEAMS-SOURCE-CONVERSATION-ROOT + timeoutMs: + expr: liveTurnTimeoutMs(env, 90000) + - set: otherConversationStart + value: + expr: state.getSnapshot().messages.length + - sendInbound: + conversation: + { id: dedupe-source-conversation, kind: channel, title: Dedupe Source Conversation } + senderId: driver + senderName: Teams QA Driver + threadId: + ref: sourceConversationRoot.id + text: "@openclaw qa msteams thread message-tool final dedupe. msteams message target: `conversation:19:dedupe-target-conversation@thread.tacv2`. exact marker: `QA-MSTEAMS-OTHER-CONVERSATION-OK`" + - call: sleep + args: + - expr: config.duplicateWindowMs + - set: otherConversationOutbound + value: + expr: "state.getSnapshot().messages.slice(otherConversationStart).filter((message) => message.direction === 'outbound' && String(message.text ?? '').includes('QA-MSTEAMS-OTHER-CONVERSATION-OK'))" + - assert: + expr: "otherConversationOutbound.length === 2 && otherConversationOutbound.some((message) => message.conversation.id === 'dedupe-source-conversation' && message.threadId === sourceConversationRoot.id) && otherConversationOutbound.some((message) => message.conversation.id === 'dedupe-target-conversation' && message.threadId === targetConversationRoot.id)" + message: + expr: "`expected source and target conversation markers; saw ${JSON.stringify(otherConversationOutbound)}`" + + - set: dmStart + value: + expr: state.getSnapshot().messages.length + - sendInbound: + conversation: { id: dedupe-dm, kind: direct, title: Dedupe DM } + senderId: driver + senderName: Teams QA Driver + replyToId: dm-quoted-parent + text: "qa msteams thread message-tool final dedupe. exact marker: `QA-MSTEAMS-DM-OK`" + - call: sleep + args: + - expr: config.duplicateWindowMs + - set: dmOutbound + value: + expr: "state.getSnapshot().messages.slice(dmStart).filter((message) => message.direction === 'outbound' && message.conversation.id === 'dedupe-dm' && String(message.text ?? '').includes('QA-MSTEAMS-DM-OK'))" + - assert: + expr: "dmOutbound.length === 1 && !dmOutbound[0]?.threadId" + message: + expr: "`expected one top-level DM marker; saw ${JSON.stringify(dmOutbound)}`" + + - set: groupStart + value: + expr: state.getSnapshot().messages.length + - sendInbound: + conversation: { id: dedupe-group, kind: group, title: Dedupe Group } + senderId: driver + senderName: Teams QA Driver + replyToId: group-quoted-parent + text: "@openclaw qa msteams thread message-tool final dedupe. exact marker: `QA-MSTEAMS-GROUP-OK`" + - call: sleep + args: + - expr: config.duplicateWindowMs + - set: groupOutbound + value: + expr: "state.getSnapshot().messages.slice(groupStart).filter((message) => message.direction === 'outbound' && message.conversation.id === 'dedupe-group' && String(message.text ?? '').includes('QA-MSTEAMS-GROUP-OK'))" + - assert: + expr: "groupOutbound.length === 1 && !groupOutbound[0]?.threadId" + message: + expr: "`expected one top-level group marker; saw ${JSON.stringify(groupOutbound)}`" + detailsExpr: "`same=${sameOutbound.length}; otherThread=${otherThreadOutbound.length}; otherConversation=${otherConversationOutbound.length}; dm=${dmOutbound.length}; group=${groupOutbound.length}; duplicateWindowMs=${config.duplicateWindowMs}`" diff --git a/src/agents/agent-tool-handler-state.test-helpers.ts b/src/agents/agent-tool-handler-state.test-helpers.ts index 5b4de931ec09..d7701a920b6a 100644 --- a/src/agents/agent-tool-handler-state.test-helpers.ts +++ b/src/agents/agent-tool-handler-state.test-helpers.ts @@ -28,6 +28,7 @@ export function createBaseToolHandlerState() { assistantMessageIndex: 0, messagingToolSentTexts: [] as string[], messagingToolSentTextsNormalized: [] as string[], + currentSourceMessagingToolSentTextsNormalized: [] as string[], messagingToolSentMediaUrls: [] as string[], messagingToolSourceReplyPayloads: [], messageToolOnlySourceReplyDelivered: false, diff --git a/src/agents/embedded-agent-message-tool-source-reply.test.ts b/src/agents/embedded-agent-message-tool-source-reply.test.ts index 51de8f4ec739..16a84fa2bdaf 100644 --- a/src/agents/embedded-agent-message-tool-source-reply.test.ts +++ b/src/agents/embedded-agent-message-tool-source-reply.test.ts @@ -4,6 +4,7 @@ import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/c import { isDeliveredMessageToolOnlySourceReplyResult, isDeliveredMessagingToolResult, + readMessageToolSourceReplyText, } from "./embedded-agent-message-tool-source-reply.js"; import { isMessagingToolDeliveryAction, @@ -503,3 +504,36 @@ describe("isDeliveredMessageToolOnlySourceReplyResult", () => { ).toBe(false); }); }); + +describe("readMessageToolSourceReplyText", () => { + it.each([ + { + name: "reply", + args: { action: "reply", message: "Visible reply" }, + expected: "Visible reply", + }, + { + name: "thread reply", + args: { action: "thread-reply", text: "Visible thread reply" }, + expected: "Visible thread reply", + }, + { + name: "poll", + args: { action: "poll", pollQuestion: "Preferred default?" }, + expected: "Preferred default?", + }, + { + name: "snake-case poll", + args: { action: "poll", poll_question: "Ready?" }, + expected: "Ready?", + }, + ])("reads $name visible text", ({ args, expected }) => { + expect(readMessageToolSourceReplyText(args)).toBe(expected); + }); + + it("ignores non-source-reply actions", () => { + expect(readMessageToolSourceReplyText({ action: "react", message: "not a reply" })).toBe( + undefined, + ); + }); +}); diff --git a/src/agents/embedded-agent-message-tool-source-reply.ts b/src/agents/embedded-agent-message-tool-source-reply.ts index e851ff911755..97f2eb5d342d 100644 --- a/src/agents/embedded-agent-message-tool-source-reply.ts +++ b/src/agents/embedded-agent-message-tool-source-reply.ts @@ -3,6 +3,7 @@ */ import { safeParseJson } from "@openclaw/normalization-core"; import { asOptionalRecord } from "@openclaw/normalization-core/record-coerce"; +import { readStringValue } from "@openclaw/normalization-core/string-coerce"; import type { SourceReplyDeliveryMode } from "../auto-reply/get-reply-options.types.js"; import { isMessageToolConversationCreateActionName, @@ -70,6 +71,24 @@ function isMessageToolSourceReplyActionName(action: unknown): boolean { return normalized === "reply" || normalized === "thread-reply" || normalized === "poll"; } +/** Read the visible text delivered by a source-reply message action. */ +export function readMessageToolSourceReplyText(args: unknown): string | undefined { + const record = asRecord(args); + if (!isMessageToolSourceReplyActionName(record.action)) { + return undefined; + } + if (normalizeStatus(record.action) === "poll") { + return readStringValue(record.pollQuestion) ?? readStringValue(record.poll_question); + } + for (const key of ["content", "message", "text", "body"]) { + const value = readStringValue(record[key]); + if (value) { + return value; + } + } + return undefined; +} + function normalizeStatus(value: unknown): string | undefined { return typeof value === "string" ? value.trim().toLowerCase() : undefined; } @@ -555,8 +574,8 @@ export function isDeliveredMessagingToolResult(params: { } /** - * Only implicit-route, non-dry-run, delivered `message.send` calls qualify. - * Explicit routes and other messaging tools are outbound side effects, not source replies. + * Only delivered message actions on the confirmed current route qualify. + * Explicit routes require an authoritative current-source marker from the action runner. */ export function isDeliveredMessageToolOnlySourceReplyResult(params: { sourceReplyDeliveryMode?: SourceReplyDeliveryMode; diff --git a/src/agents/embedded-agent-subscribe.handlers.messages.test-support.ts b/src/agents/embedded-agent-subscribe.handlers.messages.test-support.ts index df03ae7ad886..d6c104806f57 100644 --- a/src/agents/embedded-agent-subscribe.handlers.messages.test-support.ts +++ b/src/agents/embedded-agent-subscribe.handlers.messages.test-support.ts @@ -28,6 +28,17 @@ type EmbeddedSubscribeMessagesTestApi = { state: Pick, parsed: ReplyDirectiveParseResult | null | undefined, ): void; + resolveCurrentSourceMessagingToolPartial( + state: Pick< + EmbeddedAgentSubscribeState, + "currentSourceMessagingToolHeldPartial" | "currentSourceMessagingToolSentTextsNormalized" + >, + params: { + evtType: "text_delta" | "text_start" | "text_end"; + text: string; + visibleDelta: string; + }, + ): { hold: boolean; text: string }; resolveSilentReplyFallbackText(params: { text: unknown; messagingToolSentTexts: string[]; @@ -55,6 +66,20 @@ export function recordPendingAssistantReplyDirectives( getTestApi().recordPendingAssistantReplyDirectives(state, parsed); } +export function resolveCurrentSourceMessagingToolPartial( + state: Pick< + EmbeddedAgentSubscribeState, + "currentSourceMessagingToolHeldPartial" | "currentSourceMessagingToolSentTextsNormalized" + >, + params: { + evtType: "text_delta" | "text_start" | "text_end"; + text: string; + visibleDelta: string; + }, +): { hold: boolean; text: string } { + return getTestApi().resolveCurrentSourceMessagingToolPartial(state, params); +} + export function resolveSilentReplyFallbackText(params: { text: unknown; messagingToolSentTexts: string[]; diff --git a/src/agents/embedded-agent-subscribe.handlers.messages.test.ts b/src/agents/embedded-agent-subscribe.handlers.messages.test.ts index 0d1ca8ffe3eb..9b899b162d6c 100644 --- a/src/agents/embedded-agent-subscribe.handlers.messages.test.ts +++ b/src/agents/embedded-agent-subscribe.handlers.messages.test.ts @@ -15,6 +15,7 @@ import { import { buildAssistantStreamData, recordPendingAssistantReplyDirectives, + resolveCurrentSourceMessagingToolPartial, resolveSilentReplyFallbackText, } from "./embedded-agent-subscribe.handlers.messages.test-support.js"; import type { EmbeddedAgentSubscribeContext } from "./embedded-agent-subscribe.handlers.types.js"; @@ -51,6 +52,7 @@ function createMessageUpdateContext( resetAssistantMessageState?: ReturnType; debug?: ReturnType; shouldEmitPartialReplies?: boolean; + sourceReplyDeliveryMode?: "automatic" | "message_tool_only"; consumePartialReplyDirectives?: ReturnType; stripBlockTags?: ReturnType; state?: Record; @@ -65,12 +67,17 @@ function createMessageUpdateContext( params: { runId: "run-1", session: { id: "session-1" }, + ...(params.sourceReplyDeliveryMode + ? { sourceReplyDeliveryMode: params.sourceReplyDeliveryMode } + : {}), ...(params.onAgentEvent ? { onAgentEvent: params.onAgentEvent } : {}), ...(params.onPartialReply ? { onPartialReply: params.onPartialReply } : {}), }, state: { deterministicApprovalPromptPending: false, deterministicApprovalPromptSent: false, + currentSourceMessagingToolSentTextsNormalized: [], + currentSourceMessagingToolHeldPartial: undefined, reasoningStreamOpen: false, streamReasoning: false, deltaBuffer: "", @@ -158,6 +165,8 @@ function createMessageEndContext( deterministicApprovalPromptSent: false, messagingToolSentTexts: [], messagingToolSentTextsNormalized: [], + currentSourceMessagingToolSentTextsNormalized: [], + currentSourceMessagingToolHeldPartial: undefined, includeReasoning: false, streamReasoning: false, blockReplyBreak: "message_end", @@ -349,6 +358,130 @@ describe("pending assistant reply directives", () => { }); }); +describe("handleMessageUpdate current-source message-tool previews", () => { + it("holds delta-only continuation fragments and releases one full divergent snapshot", () => { + const state = { + currentSourceMessagingToolHeldPartial: undefined as string | undefined, + currentSourceMessagingToolSentTextsNormalized: ["qa-msteams-dm-ok"], + }; + + expect( + resolveCurrentSourceMessagingToolPartial(state, { + evtType: "text_delta", + text: "QA-MSTEAMS", + visibleDelta: "QA-MSTEAMS", + }), + ).toEqual({ hold: true, text: "QA-MSTEAMS" }); + expect( + resolveCurrentSourceMessagingToolPartial(state, { + evtType: "text_delta", + text: "-DM-OK", + visibleDelta: "-DM-OK", + }), + ).toEqual({ hold: true, text: "QA-MSTEAMS-DM-OK" }); + expect( + resolveCurrentSourceMessagingToolPartial(state, { + evtType: "text_delta", + text: " with more detail", + visibleDelta: " with more detail", + }), + ).toEqual({ hold: false, text: "QA-MSTEAMS-DM-OK with more detail" }); + expect(state.currentSourceMessagingToolHeldPartial).toBeUndefined(); + }); + + it("holds automatic partial prefixes and exact duplicates after source delivery", () => { + const onAgentEvent = vi.fn(); + const onPartialReply = vi.fn(); + const sentText = "QA-MSTEAMS-DM-OK"; + const context = createMessageUpdateContext({ + onAgentEvent, + onPartialReply, + sourceReplyDeliveryMode: "automatic", + state: { + currentSourceMessagingToolSentTextsNormalized: [sentText.toLowerCase()], + }, + }); + + updateMessage( + context, + createTextUpdateEvent({ + type: "text_delta", + text: "QA-MSTEAMS", + id: "msg_source_duplicate", + }), + ); + updateMessage( + context, + createTextUpdateEvent({ + type: "text_end", + text: sentText, + id: "msg_source_duplicate", + }), + ); + + expect(onAgentEvent).toHaveBeenCalledTimes(1); + expect(onPartialReply).not.toHaveBeenCalled(); + }); + + it("releases the full cumulative snapshot when automatic text diverges", () => { + const onPartialReply = vi.fn(); + const sentText = "QA-MSTEAMS-DM-OK"; + const context = createMessageUpdateContext({ + onPartialReply, + sourceReplyDeliveryMode: "automatic", + state: { + currentSourceMessagingToolSentTextsNormalized: [sentText.toLowerCase()], + }, + }); + + updateMessage( + context, + createTextUpdateEvent({ + type: "text_delta", + text: "QA-MSTEAMS", + id: "msg_source_diverges", + }), + ); + updateMessage( + context, + createTextUpdateEvent({ + type: "text_end", + text: `${sentText} with more detail`, + id: "msg_source_diverges", + }), + ); + + expect(onPartialReply).toHaveBeenCalledTimes(1); + expect(onPartialReply).toHaveBeenCalledWith( + expect.objectContaining({ text: `${sentText} with more detail` }), + ); + }); + + it("keeps unrelated automatic partial text visible", () => { + const onPartialReply = vi.fn(); + const context = createMessageUpdateContext({ + onPartialReply, + sourceReplyDeliveryMode: "automatic", + state: { + currentSourceMessagingToolSentTextsNormalized: ["qa-msteams-dm-ok"], + }, + }); + + updateMessage( + context, + createTextUpdateEvent({ + type: "text_end", + text: "A genuinely different answer", + id: "msg_source_different", + }), + ); + + expect(onPartialReply).toHaveBeenCalledWith( + expect.objectContaining({ text: "A genuinely different answer" }), + ); + }); +}); + describe("handleMessageUpdate text signatures", () => { it("uses incremental text deltas for unphased OpenAI Responses streams", () => { const onAgentEvent = vi.fn(); diff --git a/src/agents/embedded-agent-subscribe.handlers.messages.ts b/src/agents/embedded-agent-subscribe.handlers.messages.ts index debeee775e91..f08c84610e3e 100644 --- a/src/agents/embedded-agent-subscribe.handlers.messages.ts +++ b/src/agents/embedded-agent-subscribe.handlers.messages.ts @@ -301,6 +301,36 @@ function hasMessageToolOnlySourceDelivery(ctx: EmbeddedAgentSubscribeContext): b ); } +function resolveCurrentSourceMessagingToolPartial( + state: Pick< + EmbeddedAgentSubscribeState, + "currentSourceMessagingToolHeldPartial" | "currentSourceMessagingToolSentTextsNormalized" + >, + params: { + evtType: "text_delta" | "text_start" | "text_end"; + text: string; + visibleDelta: string; + }, +): { hold: boolean; text: string } { + const held = state.currentSourceMessagingToolHeldPartial; + const text = + held && params.evtType === "text_delta" && !params.text.startsWith(held) + ? `${held}${params.visibleDelta || params.text}` + : params.text; + const normalized = normalizeTextForComparison(text); + if (!normalized) { + state.currentSourceMessagingToolHeldPartial = undefined; + return { hold: false, text }; + } + // A confirmed current-source tool send already made this prefix visible. + // Hold it until the assistant either repeats the sent text or diverges with new content. + const hold = state.currentSourceMessagingToolSentTextsNormalized.some( + (sentText) => sentText === normalized || sentText.startsWith(normalized), + ); + state.currentSourceMessagingToolHeldPartial = hold ? text : undefined; + return { hold, text }; +} + function appendBlockReplyChunk(ctx: EmbeddedAgentSubscribeContext, chunk: string) { if (ctx.blockChunker) { ctx.blockChunker.append(chunk); @@ -1157,14 +1187,23 @@ export function handleMessageUpdate( } if (shouldEmit) { + const currentSourcePartial = + ctx.params.sourceReplyDeliveryMode !== "message_tool_only" + ? resolveCurrentSourceMessagingToolPartial(ctx.state, { + evtType, + text: cleanedText, + visibleDelta, + }) + : { hold: false, text: cleanedText }; + const releaseHeldSnapshot = currentSourcePartial.text !== cleanedText; const data = buildAssistantStreamData({ - text: cleanedText, - delta: deltaText, - replace, + text: currentSourcePartial.text, + delta: releaseHeldSnapshot ? currentSourcePartial.text : deltaText, + replace: releaseHeldSnapshot || replace, mediaUrls, phase: deliveryPhase ?? assistantPhase, }); - ctx.emitAssistantStreamData(data, { emitPartialReply: true }); + ctx.emitAssistantStreamData(data, { emitPartialReply: !currentSourcePartial.hold }); ctx.state.emittedAssistantUpdate = true; } } else if (shouldPersistRawStreamText) { @@ -1204,6 +1243,7 @@ if (process.env.VITEST || process.env.NODE_ENV === "test") { ] = { buildAssistantStreamData, recordPendingAssistantReplyDirectives, + resolveCurrentSourceMessagingToolPartial, resolveSilentReplyFallbackText, }; } diff --git a/src/agents/embedded-agent-subscribe.handlers.tools.media.test.ts b/src/agents/embedded-agent-subscribe.handlers.tools.media.test.ts index 9e04b8e4fd58..e7bb9dfbd2c4 100644 --- a/src/agents/embedded-agent-subscribe.handlers.tools.media.test.ts +++ b/src/agents/embedded-agent-subscribe.handlers.tools.media.test.ts @@ -40,6 +40,7 @@ function createMockContext(overrides?: { pendingToolAudioAsVoice: false, messagingToolSentTexts: [], messagingToolSentTextsNormalized: [], + currentSourceMessagingToolSentTextsNormalized: [], messagingToolSentMediaUrls: [], messagingToolSourceReplyPayloads: [], messageToolOnlySourceReplyDelivered: false, diff --git a/src/agents/embedded-agent-subscribe.handlers.tools.test.ts b/src/agents/embedded-agent-subscribe.handlers.tools.test.ts index 15dc7a5511ca..2618532b1368 100644 --- a/src/agents/embedded-agent-subscribe.handlers.tools.test.ts +++ b/src/agents/embedded-agent-subscribe.handlers.tools.test.ts @@ -191,6 +191,7 @@ function createTestContext(): { replayState: { replayInvalid: false, hadPotentialSideEffects: false }, messagingToolSentTexts: [], messagingToolSentTextsNormalized: [], + currentSourceMessagingToolSentTextsNormalized: [], messagingToolSentMediaUrls: [], messagingToolSourceReplyPayloads: [], messageToolOnlySourceReplyDelivered: false, @@ -1633,6 +1634,43 @@ describe("handleToolExecutionEnd mutating failure recovery", () => { ]); }); + it("records preview suppression text only for confirmed current-source sends", async () => { + const { ctx } = createTestContext(); + ctx.params.sourceReplyDeliveryMode = "automatic"; + + await executeTool(ctx, { + toolName: "message", + toolCallId: "tool-message-other-route", + args: { + action: "send", + provider: "telegram", + to: "chat-other", + text: "Other route text", + }, + isError: false, + result: { details: { ok: true } }, + }); + await executeTool(ctx, { + toolName: "message", + toolCallId: "tool-message-current-source", + args: { + action: "send", + provider: "telegram", + to: "chat-source", + text: "QA-MSTEAMS-DM-OK", + }, + isError: false, + result: { + details: { + ok: true, + sourceReplyRoute: "current-source", + }, + }, + }); + + expect(ctx.state.currentSourceMessagingToolSentTextsNormalized).toEqual(["qa-msteams-dm-ok"]); + }); + it("records rich-content delivery when visible text is blank", async () => { const { ctx } = createTestContext(); const toolCallId = "tool-message-rich-content"; @@ -1691,6 +1729,93 @@ describe("handleToolExecutionEnd mutating failure recovery", () => { ]); }); + it.each([ + { + name: "reply", + args: { + action: "reply", + provider: "telegram", + target: "chat-reply", + message: "Visible reply", + }, + result: { + ok: true, + messageId: "message-reply", + details: { sourceReplyRoute: "current-source" }, + }, + expected: "visible reply", + }, + { + name: "poll", + args: { + action: "poll", + provider: "telegram", + target: "chat-poll", + pollQuestion: "Preferred default?", + pollOption: ["Tell me right away", "Only important"], + }, + result: { + ok: true, + pollId: "poll-1", + details: { sourceReplyRoute: "current-source" }, + }, + expected: "preferred default?", + }, + ])("records confirmed current-source $name text for preview dedupe", async (testCase) => { + const { ctx } = createTestContext(); + ctx.params.sourceReplyDeliveryMode = "automatic"; + + await executeTool(ctx, { + toolName: "message", + toolCallId: `tool-message-current-source-${testCase.name}`, + args: testCase.args, + isError: false, + result: testCase.result, + }); + + expect(ctx.state.currentSourceMessagingToolSentTextsNormalized).toEqual([testCase.expected]); + expect(ctx.state.messageToolOnlySourceReplyDelivered).toBe(true); + expect(ctx.state.messagingToolSentTexts).toEqual([]); + }); + + it.each([ + { + name: "reply", + args: { + action: "reply", + provider: "telegram", + target: "chat-reply", + message: "Visible reply", + }, + result: { ok: true, messageId: "message-reply" }, + }, + { + name: "poll", + args: { + action: "poll", + provider: "telegram", + target: "chat-poll", + pollQuestion: "Preferred default?", + pollOption: ["Tell me right away", "Only important"], + }, + result: { ok: true, pollId: "poll-1" }, + }, + ])("does not record off-route $name text for preview dedupe", async (testCase) => { + const { ctx } = createTestContext(); + ctx.params.sourceReplyDeliveryMode = "automatic"; + + await executeTool(ctx, { + toolName: "message", + toolCallId: `tool-message-off-route-${testCase.name}`, + args: testCase.args, + isError: false, + result: testCase.result, + }); + + expect(ctx.state.currentSourceMessagingToolSentTextsNormalized).toEqual([]); + expect(ctx.state.messageToolOnlySourceReplyDelivered).toBe(false); + }); + it("records conversation creation target evidence", async () => { const { ctx } = createTestContext(); const toolCallId = "tool-message-thread-create-target"; diff --git a/src/agents/embedded-agent-subscribe.handlers.tools.ts b/src/agents/embedded-agent-subscribe.handlers.tools.ts index b40868f82161..7e79bc644d1b 100644 --- a/src/agents/embedded-agent-subscribe.handlers.tools.ts +++ b/src/agents/embedded-agent-subscribe.handlers.tools.ts @@ -55,6 +55,7 @@ import { normalizeTextForComparison } from "./embedded-agent-helpers.js"; import { isDeliveredMessageToolOnlySourceReplyResult, isDeliveredMessagingToolResult, + readMessageToolSourceReplyText, } from "./embedded-agent-message-tool-source-reply.js"; import { isMessagingTool, @@ -1571,23 +1572,32 @@ export async function handleToolExecutionEnd( }); ctx.trimMessagingToolSent(); } + const deliveredCurrentSourceReply = + didDeliverMessagingResult && + isDeliveredMessageToolOnlySourceReplyResult({ + sourceReplyDeliveryMode: ctx.params.sourceReplyDeliveryMode, + toolName, + args: startArgs, + result, + isError: isToolError, + }); + if (deliveredCurrentSourceReply) { + ctx.state.messageToolOnlySourceReplyDelivered = true; + const sourceReplyText = readMessageToolSourceReplyText(startArgs); + const normalizedSourceReplyText = sourceReplyText + ? normalizeTextForComparison(sourceReplyText) + : ""; + if (normalizedSourceReplyText) { + ctx.state.currentSourceMessagingToolSentTextsNormalized.push(normalizedSourceReplyText); + ctx.trimMessagingToolSent(); + } + ctx.params.onDeliveredMessageToolOnlySourceReply?.(); + } if (didDeliverMessagingResult && isMessagingSend) { if (committedMediaUrls.length > 0) { ctx.state.messagingToolSentMediaUrls.push(...committedMediaUrls); ctx.trimMessagingToolSent(); } - if ( - isDeliveredMessageToolOnlySourceReplyResult({ - sourceReplyDeliveryMode: ctx.params.sourceReplyDeliveryMode, - toolName, - args: startArgs, - result, - isError: isToolError, - }) - ) { - ctx.state.messageToolOnlySourceReplyDelivered = true; - ctx.params.onDeliveredMessageToolOnlySourceReply?.(); - } const sourceReplyPayload = extractMessagingToolSourceReplyPayload(result); if (sourceReplyPayload) { ctx.state.messagingToolSourceReplyPayloads.push(sourceReplyPayload); diff --git a/src/agents/embedded-agent-subscribe.handlers.types.ts b/src/agents/embedded-agent-subscribe.handlers.types.ts index 25f7c2c3e192..5eda9f1e813b 100644 --- a/src/agents/embedded-agent-subscribe.handlers.types.ts +++ b/src/agents/embedded-agent-subscribe.handlers.types.ts @@ -170,6 +170,8 @@ export type EmbeddedAgentSubscribeState = { messagingToolSentTexts: string[]; messagingToolSentTextsNormalized: string[]; + currentSourceMessagingToolSentTextsNormalized: string[]; + currentSourceMessagingToolHeldPartial?: string; messagingToolSentTargets: MessagingToolSend[]; heartbeatToolResponse?: HeartbeatToolResponse; messagingToolSentMediaUrls: string[]; @@ -338,6 +340,7 @@ type ToolHandlerState = Pick< | "replayState" | "messagingToolSentTexts" | "messagingToolSentTextsNormalized" + | "currentSourceMessagingToolSentTextsNormalized" | "messagingToolSentMediaUrls" | "messagingToolSourceReplyPayloads" | "messageToolOnlySourceReplyDelivered" diff --git a/src/agents/embedded-agent-subscribe.ts b/src/agents/embedded-agent-subscribe.ts index 75731ecdb392..e1891f35ff44 100644 --- a/src/agents/embedded-agent-subscribe.ts +++ b/src/agents/embedded-agent-subscribe.ts @@ -258,6 +258,8 @@ export function subscribeEmbeddedAgentSession(params: SubscribeEmbeddedAgentSess pendingEventChain: null, messagingToolSentTexts: [], messagingToolSentTextsNormalized: [], + currentSourceMessagingToolSentTextsNormalized: [], + currentSourceMessagingToolHeldPartial: undefined, messagingToolSentTargets: [], heartbeatToolResponse: undefined, messagingToolSentMediaUrls: [], @@ -477,6 +479,7 @@ export function subscribeEmbeddedAgentSession(params: SubscribeEmbeddedAgentSess state.partialBlockState.pendingTagFragment = undefined; state.lastStreamedAssistant = undefined; state.lastStreamedAssistantCleaned = undefined; + state.currentSourceMessagingToolHeldPartial = undefined; state.emittedAssistantUpdate = false; state.lastBlockReplyText = undefined; state.lastStreamedReasoning = undefined; @@ -567,6 +570,7 @@ export function subscribeEmbeddedAgentSession(params: SubscribeEmbeddedAgentSess // to support commit logic but not used for suppression (avoiding lost messages on tool failure). // These tools can send messages via sendMessage/threadReply actions (or sessions_send with message). const MAX_MESSAGING_SENT_TEXTS = 200; + const MAX_CURRENT_SOURCE_MESSAGING_SENT_TEXTS = 200; const MAX_MESSAGING_SENT_TARGETS = 200; const MAX_MESSAGING_SENT_MEDIA_URLS = 200; const MAX_MESSAGING_SOURCE_REPLY_PAYLOADS = 200; @@ -576,6 +580,15 @@ export function subscribeEmbeddedAgentSession(params: SubscribeEmbeddedAgentSess messagingToolSentTexts.splice(0, overflow); messagingToolSentTextsNormalized.splice(0, overflow); } + if ( + state.currentSourceMessagingToolSentTextsNormalized.length > + MAX_CURRENT_SOURCE_MESSAGING_SENT_TEXTS + ) { + const overflow = + state.currentSourceMessagingToolSentTextsNormalized.length - + MAX_CURRENT_SOURCE_MESSAGING_SENT_TEXTS; + state.currentSourceMessagingToolSentTextsNormalized.splice(0, overflow); + } if (messagingToolSentTargets.length > MAX_MESSAGING_SENT_TARGETS) { const overflow = messagingToolSentTargets.length - MAX_MESSAGING_SENT_TARGETS; messagingToolSentTargets.splice(0, overflow); @@ -1328,6 +1341,7 @@ export function subscribeEmbeddedAgentSession(params: SubscribeEmbeddedAgentSess } messagingToolSentTexts.length = 0; messagingToolSentTextsNormalized.length = 0; + state.currentSourceMessagingToolSentTextsNormalized.length = 0; messagingToolSentTargets.length = 0; messagingToolSentMediaUrls.length = 0; pendingMessagingTexts.clear(); diff --git a/src/auto-reply/reply/reply-payloads.test.ts b/src/auto-reply/reply/reply-payloads.test.ts index 03627c33b78e..2158d7bca949 100644 --- a/src/auto-reply/reply/reply-payloads.test.ts +++ b/src/auto-reply/reply/reply-payloads.test.ts @@ -216,6 +216,38 @@ describe("shouldDedupeMessagingToolRepliesForRoute", () => { ).toBe(false); }); + it("matches a Teams send resolved to the originating DM conversation", () => { + expect( + shouldDedupeMessagingToolRepliesForRoute({ + messageProvider: "msteams", + originatingTo: "conversation:19:dm-current@thread.v2", + messagingToolSentTargets: [ + { + tool: "message", + provider: "msteams", + to: "conversation:19:dm-current@thread.v2", + }, + ], + }), + ).toBe(true); + }); + + it("does not match a Teams user alias resolved to a different DM conversation", () => { + expect( + shouldDedupeMessagingToolRepliesForRoute({ + messageProvider: "msteams", + originatingTo: "conversation:19:dm-current@thread.v2", + messagingToolSentTargets: [ + { + tool: "message", + provider: "msteams", + to: "conversation:19:dm-newer@thread.v2", + }, + ], + }), + ).toBe(false); + }); + it("matches when only one side carries the account id", () => { expect( shouldDedupeMessagingToolRepliesForRoute({