fix(msteams): thread attachment sends into channel reply threads (#88836)

MoerAI · MoerAI · commit a94a78efa173 · 2026-06-01T10:12:34.000+09:00
Microsoft Teams attachment branches (SharePoint native file card and OneDrive file-link fallback) call sendProactiveActivityRaw without a thread root, so channel replies configured with replyStyle: 'thread' arrive as new top-level messages even when the originating message lived in a thread. Text and inline-media sends already preserve replyStyle via sendMSTeamsMessages -> sendMSTeamsActivityWithReference, so the gap is specific to the file-card and file-link branches. Adds a resolveProactiveThreadActivityId helper that mirrors messenger.ts thread routing: only channel refs with replyStyle === 'thread' get a thread root, with the same threadId ?? activityId fallback for older stored refs. Threads the result through sendProactiveActivityRaw -> sendMSTeamsActivityWithReference at both attachment call sites. This follows the conservative direction in clawsweeper's review on #88836: "Keep this issue open and make the Teams file-card/file-link attachment branches use the same thread-aware delivery path as text and inline media, with regression tests for replyStyle: 'thread' and replyStyle: 'top-level'."
diff --git a/extensions/msteams/src/send.test.ts b/extensions/msteams/src/send.test.ts
@@ -18,6 +18,7 @@ const mockState = vi.hoisted(() => ({
   updateMSTeamsActivityWithReference: vi.fn(async () => ({ id: "updated" })),
   deleteMSTeamsActivityWithReference: vi.fn(async () => {}),
   uploadAndShareSharePoint: vi.fn(),
+  uploadAndShareOneDrive: vi.fn(),
   getDriveItemProperties: vi.fn(),
   buildTeamsFileInfoCard: vi.fn(),
   createMSTeamsTokenProvider: vi.fn(),
@@ -85,7 +86,7 @@ vi.mock("./runtime.js", () => ({
 vi.mock("./graph-upload.js", () => ({
   uploadAndShareSharePoint: mockState.uploadAndShareSharePoint,
   getDriveItemProperties: mockState.getDriveItemProperties,
-  uploadAndShareOneDrive: vi.fn(),
+  uploadAndShareOneDrive: mockState.uploadAndShareOneDrive,
 }));
 
 vi.mock("./graph-chat.js", () => ({
@@ -154,16 +155,21 @@ function createSharePointSendContext(params: {
   conversationId: string;
   graphChatId: string | null;
   siteId: string;
+  // Override `replyStyle` and the conversation ref shape for #88836 channel-thread coverage.
+  // Defaults preserve the historical groupChat / top-level shape used by existing tests.
+  replyStyle?: "thread" | "top-level";
+  conversationType?: "personal" | "groupChat" | "channel";
+  ref?: Record<string, unknown>;
 }) {
   return {
     app: createMockApp(),
     appId: "app-id",
     conversationId: params.conversationId,
     graphChatId: params.graphChatId,
-    ref: {},
+    ref: params.ref ?? {},
     log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
-    conversationType: "groupChat" as const,
-    replyStyle: "top-level" as const,
+    conversationType: params.conversationType ?? ("groupChat" as const),
+    replyStyle: params.replyStyle ?? ("top-level" as const),
     sdkCloudOptions: { cloud: "Public" as const },
     tokenProvider: { getAccessToken: vi.fn(async () => "token") },
     mediaMaxBytes: 8 * 1024 * 1024,
@@ -443,6 +449,208 @@ describe("sendMessageMSTeams", () => {
     expect(uploadPayload.chatId).toBe(botFrameworkConversationId);
     expect(uploadPayload.siteId).toBe("site-456");
   });
+
+  // #88836: attachment branches (SharePoint native file card, OneDrive file link)
+  // previously sent raw proactive activities without thread context, so channel
+  // replies with `replyStyle: "thread"` lost their thread placement.
+  it("threads SharePoint native file card into the channel thread when replyStyle is 'thread' (#88836)", async () => {
+    const channelConversationId = "19:channel-thread@thread.skype";
+    const threadRootId = "1700000000000";
+
+    mockState.resolveMSTeamsSendContext.mockResolvedValue(
+      createSharePointSendContext({
+        conversationId: channelConversationId,
+        graphChatId: "19:graph-chat@thread.skype",
+        siteId: "site-thread",
+        replyStyle: "thread",
+        conversationType: "channel",
+        ref: {
+          conversation: { id: channelConversationId, conversationType: "channel" },
+          threadId: threadRootId,
+          channelId: "msteams",
+        },
+      }),
+    );
+    mockSharePointPdfUpload({
+      bufferSize: 100,
+      fileName: "thread-doc.pdf",
+      itemId: "item-thread",
+      uniqueId: "{GUID-THREAD}",
+    });
+
+    await sendMessageMSTeams({
+      cfg: {} as OpenClawConfig,
+      to: "conversation:19:channel-thread@thread.skype",
+      text: "threaded file",
+      mediaUrl: "https://example.com/thread-doc.pdf",
+    });
+
+    // The proactive send must carry threadActivityId so the attachment lands
+    // in the originating Teams thread rather than as a top-level message.
+    const sendArgs = mockState.sendMSTeamsActivityWithReference.mock.calls[0];
+    const sendOptions = sendArgs?.[3] as { threadActivityId?: string } | undefined;
+    expect(sendOptions?.threadActivityId).toBe(threadRootId);
+  });
+
+  it("falls back to ref.activityId for thread root when ref.threadId is absent (#88836)", async () => {
+    const channelConversationId = "19:legacy-channel@thread.skype";
+    const legacyActivityId = "1690000000000";
+
+    mockState.resolveMSTeamsSendContext.mockResolvedValue(
+      createSharePointSendContext({
+        conversationId: channelConversationId,
+        graphChatId: null,
+        siteId: "site-legacy",
+        replyStyle: "thread",
+        conversationType: "channel",
+        ref: {
+          conversation: { id: channelConversationId, conversationType: "channel" },
+          activityId: legacyActivityId,
+          channelId: "msteams",
+        },
+      }),
+    );
+    mockSharePointPdfUpload({
+      bufferSize: 100,
+      fileName: "legacy-doc.pdf",
+      itemId: "item-legacy",
+      uniqueId: "{GUID-LEGACY}",
+    });
+
+    await sendMessageMSTeams({
+      cfg: {} as OpenClawConfig,
+      to: "conversation:19:legacy-channel@thread.skype",
+      text: "legacy ref",
+      mediaUrl: "https://example.com/legacy-doc.pdf",
+    });
+
+    const sendArgs = mockState.sendMSTeamsActivityWithReference.mock.calls[0];
+    const sendOptions = sendArgs?.[3] as { threadActivityId?: string } | undefined;
+    expect(sendOptions?.threadActivityId).toBe(legacyActivityId);
+  });
+
+  it("does not thread the SharePoint file card when replyStyle is 'top-level' (#88836)", async () => {
+    const channelConversationId = "19:top-level@thread.skype";
+
+    mockState.resolveMSTeamsSendContext.mockResolvedValue(
+      createSharePointSendContext({
+        conversationId: channelConversationId,
+        graphChatId: null,
+        siteId: "site-top",
+        replyStyle: "top-level",
+        conversationType: "channel",
+        ref: {
+          conversation: { id: channelConversationId, conversationType: "channel" },
+          threadId: "should-not-be-used",
+          channelId: "msteams",
+        },
+      }),
+    );
+    mockSharePointPdfUpload({
+      bufferSize: 100,
+      fileName: "top-doc.pdf",
+      itemId: "item-top",
+      uniqueId: "{GUID-TOP}",
+    });
+
+    await sendMessageMSTeams({
+      cfg: {} as OpenClawConfig,
+      to: "conversation:19:top-level@thread.skype",
+      text: "top-level file",
+      mediaUrl: "https://example.com/top-doc.pdf",
+    });
+
+    // Explicit top-level intent must not silently thread even when the ref has
+    // a threadId; preserves the existing replyStyle contract.
+    const sendArgs = mockState.sendMSTeamsActivityWithReference.mock.calls[0];
+    const sendOptions = sendArgs?.[3] as { threadActivityId?: string } | undefined;
+    expect(sendOptions?.threadActivityId).toBeUndefined();
+  });
+
+  it("does not thread when the ref is a personal/group chat even with replyStyle 'thread' (#88836)", async () => {
+    // Defensive: messenger.ts only threads channel refs. The attachment path
+    // must mirror that owner-boundary so personal/group chats never get a
+    // threadActivityId suffix even if a stored ref accidentally carries one.
+    const personalConversationId = "19:personal@unq.gbl.spaces";
+
+    mockState.resolveMSTeamsSendContext.mockResolvedValue(
+      createSharePointSendContext({
+        conversationId: personalConversationId,
+        graphChatId: null,
+        siteId: "site-personal",
+        replyStyle: "thread",
+        conversationType: "personal",
+        ref: {
+          conversation: { id: personalConversationId, conversationType: "personal" },
+          threadId: "should-not-be-used",
+          channelId: "msteams",
+        },
+      }),
+    );
+    mockSharePointPdfUpload({
+      bufferSize: 100,
+      fileName: "personal-doc.pdf",
+      itemId: "item-personal",
+      uniqueId: "{GUID-PERSONAL}",
+    });
+
+    await sendMessageMSTeams({
+      cfg: {} as OpenClawConfig,
+      to: "conversation:19:personal@unq.gbl.spaces",
+      text: "personal file",
+      mediaUrl: "https://example.com/personal-doc.pdf",
+    });
+
+    const sendArgs = mockState.sendMSTeamsActivityWithReference.mock.calls[0];
+    const sendOptions = sendArgs?.[3] as { threadActivityId?: string } | undefined;
+    expect(sendOptions?.threadActivityId).toBeUndefined();
+  });
+
+  it("threads OneDrive file-link fallback into the channel thread when replyStyle is 'thread' (#88836)", async () => {
+    // No SharePoint site configured -> OneDrive markdown link fallback path.
+    // The bug report's exact symptom lives here too: the file-link branch
+    // also called sendProactiveActivityRaw without thread context.
+    const channelConversationId = "19:channel-onedrive@thread.skype";
+    const threadRootId = "1701111111111";
+
+    mockState.resolveMSTeamsSendContext.mockResolvedValue(
+      createSharePointSendContext({
+        conversationId: channelConversationId,
+        graphChatId: null,
+        siteId: "", // empty siteId triggers OneDrive fallback
+        replyStyle: "thread",
+        conversationType: "channel",
+        ref: {
+          conversation: { id: channelConversationId, conversationType: "channel" },
+          threadId: threadRootId,
+          channelId: "msteams",
+        },
+      }),
+    );
+    mockState.loadOutboundMediaFromUrl.mockResolvedValueOnce({
+      buffer: Buffer.alloc(100, "pdf"),
+      contentType: "application/pdf",
+      fileName: "onedrive-doc.pdf",
+      kind: "file",
+    });
+    mockState.requiresFileConsent.mockReturnValue(false);
+    mockState.uploadAndShareOneDrive.mockResolvedValueOnce({
+      itemId: "od-item",
+      shareUrl: "https://onedrive.example.com/share/onedrive-doc.pdf",
+      name: "onedrive-doc.pdf",
+    });
+
+    await sendMessageMSTeams({
+      cfg: {} as OpenClawConfig,
+      to: "conversation:19:channel-onedrive@thread.skype",
+      text: "onedrive link",
+      mediaUrl: "https://example.com/onedrive-doc.pdf",
+    });
+
+    const sendArgs = mockState.sendMSTeamsActivityWithReference.mock.calls[0];
+    const sendOptions = sendArgs?.[3] as { threadActivityId?: string } | undefined;
+    expect(sendOptions?.threadActivityId).toBe(threadRootId);
+  });
 });
 
 describe("MSTeams continueConversation failure handling", () => {
diff --git a/extensions/msteams/src/send.ts b/extensions/msteams/src/send.ts
@@ -307,6 +307,9 @@ export async function sendMessageMSTeams(
           ref,
           activity,
           serviceUrlBoundary: sdkCloudOptions,
+          // Thread the file card into the originating channel thread when
+          // the ref carries `replyStyle: "thread"` (#88836).
+          threadActivityId: resolveProactiveThreadActivityId(ctx),
         });
 
         log.info("sent native file card", {
@@ -351,6 +354,9 @@ export async function sendMessageMSTeams(
         ref,
         activity,
         serviceUrlBoundary: sdkCloudOptions,
+        // Same thread-aware delivery as the SharePoint native file card path
+        // so the OneDrive file-link fallback also lands in-thread (#88836).
+        threadActivityId: resolveProactiveThreadActivityId(ctx),
       });
 
       log.info("sent message with OneDrive file link", {
@@ -446,19 +452,47 @@ type ProactiveActivityParams = {
   activity: Record<string, unknown>;
   errorPrefix: string;
   serviceUrlBoundary: MSTeamsProactiveContext["sdkCloudOptions"];
+  // Forward channel thread root for proactive attachment sends so file-card /
+  // file-link branches land in the originating Teams thread when the channel
+  // ref has `replyStyle: "thread"`. Mirrors messenger.ts thread routing
+  // (#88836). Plain personal/group chats and explicit `top-level` sends pass
+  // undefined so we never silently thread a non-channel ref.
+  threadActivityId?: string;
 };
 
+/**
+ * Resolve the channel thread root for proactive attachment sends (#88836).
+ * Returns undefined for non-channel refs or `replyStyle: "top-level"` so
+ * personal/group chats and explicit top-level sends never get a thread
+ * suffix. The thread root falls back to `activityId` for older stored refs
+ * that pre-date `threadId`, matching the resolution used in messenger.ts.
+ */
+function resolveProactiveThreadActivityId(
+  ctx: MSTeamsProactiveContext,
+): string | undefined {
+  if (ctx.replyStyle !== "thread") {
+    return undefined;
+  }
+  const isChannel = ctx.ref.conversation?.conversationType === "channel";
+  if (!isChannel) {
+    return undefined;
+  }
+  return ctx.ref.threadId ?? ctx.ref.activityId;
+}
+
 type ProactiveActivityRawParams = Omit<ProactiveActivityParams, "errorPrefix">;
 
 async function sendProactiveActivityRaw({
   app,
   ref,
   activity,
   serviceUrlBoundary,
+  threadActivityId,
 }: ProactiveActivityRawParams): Promise<string> {
   const baseRef = buildConversationReference(ref);
   const response = await sendMSTeamsActivityWithReference(app, baseRef, activity, {
     serviceUrlBoundary,
+    threadActivityId,
   });
   return extractMessageId(response) ?? "unknown";
 }
