fix(msteams): gate graphMediaFallback on a group-chat allowlist, not not-channel

packwood · claude · packwood · commit 3ec3e9ac61dc · 2026-07-06T22:45:57.000-04:00
The prior scope fix excluded channel conversations with a denylist, but personal DMs also arrive as conversationType 'personal' and can carry Graph-compatible 19:...@unq.gbl.spaces chat IDs that isBotFrameworkPersonalChatId does not catch — so a documented group-chat option silently changed DM Graph-lookup behavior. Gate on conversationType === 'groupChat' (normalized) instead. Adds a regression: personal + Graph-compatible 19: ID + flag on does not call Graph. Addresses ClawSweeper P1 on #101096. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diff --git a/extensions/msteams/src/monitor-handler/inbound-media.test.ts b/extensions/msteams/src/monitor-handler/inbound-media.test.ts
@@ -177,6 +177,33 @@ describe("resolveMSTeamsInboundMedia graph fallback trigger", () => {
     expect(buildMSTeamsGraphMessageUrls).not.toHaveBeenCalled();
   });
 
+  it("does NOT apply graphMediaFallback to personal chats with Graph-compatible 19: IDs", async () => {
+    vi.mocked(downloadMSTeamsAttachments).mockResolvedValue([]);
+    // A personal DM can carry a `19:...@unq.gbl.spaces` chat ID that is NOT a
+    // Bot Framework personal ID, so isBotFrameworkPersonalChatId does not catch
+    // it. The trigger must be a group-chat allowlist, not "not channel", or the
+    // documented group-chat option would silently change DM Graph behavior.
+    vi.mocked(extractMSTeamsHtmlAttachmentIds).mockReturnValueOnce([]);
+    vi.mocked(downloadMSTeamsGraphMedia).mockClear();
+    vi.mocked(buildMSTeamsGraphMessageUrls).mockClear();
+
+    await resolveMSTeamsInboundMedia({
+      ...baseParams,
+      conversationType: "personal",
+      conversationId: "19:user_bot@unq.gbl.spaces",
+      graphMediaFallback: true,
+      attachments: [
+        {
+          contentType: "text/html",
+          content: "<div>please read the attached file</div>",
+        },
+      ],
+    });
+
+    expect(downloadMSTeamsGraphMedia).not.toHaveBeenCalled();
+    expect(buildMSTeamsGraphMessageUrls).not.toHaveBeenCalled();
+  });
+
   it("does NOT apply graphMediaFallback to channel conversations", async () => {
     vi.mocked(downloadMSTeamsAttachments).mockResolvedValue([]);
     // Channel Graph URL building still has known team-GUID and reply-URL
diff --git a/extensions/msteams/src/monitor-handler/inbound-media.ts b/extensions/msteams/src/monitor-handler/inbound-media.ts
@@ -136,19 +136,22 @@ export async function resolveMSTeamsInboundMedia(params: {
     // Teams strips `<attachment id>` tags from the HTML delivered to bots in
     // group chats, so real file attachments can arrive with an empty extracted
     // ID list (#89594). With `channels.msteams.graphMediaFallback` enabled,
-    // attempt the Graph fetch for any group-chat message that carried an HTML
+    // attempt the Graph fetch for a group-chat message that carried an HTML
     // attachment and produced no media — at the cost of one Graph message
     // lookup per such message. The default stays off so mention-only messages
-    // don't generate spurious lookups (#58617). Channel conversations are
-    // excluded: the channel Graph URL builder still has known team-GUID and
-    // reply-URL defects, so widening the trigger there would route channel
-    // traffic into a path that can 400/404 (see the channel attachment repair
-    // tracked alongside #89594).
-    const isChannelConversation = conversationType.trim().toLowerCase() === "channel";
+    // don't generate spurious lookups (#58617).
+    //
+    // The widened trigger is an allowlist on the group-chat conversation type,
+    // not "everything except channel": personal DMs also arrive as `personal`
+    // and can carry Graph-compatible `19:...` chat IDs that are not caught by
+    // `isBotFrameworkPersonalChatId`, so a denylist would change DM behavior
+    // too. Channel conversations stay out because their Graph URL builder has
+    // separate known team-GUID and reply-URL defects (broader channel repair).
+    const isGroupChatConversation = conversationType.trim().toLowerCase() === "groupchat";
     const hasGraphFallbackCandidate =
       hasHtmlFileAttachment ||
       (params.graphMediaFallback === true &&
-        !isChannelConversation &&
+        isGroupChatConversation &&
         attachments.some(
           (att) =>
             typeof att.contentType === "string" &&
diff --git a/src/config/types.msteams.ts b/src/config/types.msteams.ts
@@ -165,8 +165,9 @@ export type MSTeamsConfig = {
    * Fetch group-chat message media via Microsoft Graph even when the Bot
    * Framework payload contains no `<attachment id>` references. Teams strips
    * those tags from group-chat HTML, so file attachments are otherwise
-   * invisible to the bot. Applies to group chats only; channel (team-scope)
-   * messages keep the tag-gated fallback. Requires `Chat.Read.All` plus
+   * invisible to the bot. Applies to group chats only (conversationType
+   * `groupChat`); personal DMs and channel messages keep the tag-gated
+   * fallback. Requires `Chat.Read.All` plus
    * `Files.Read.All` Graph Application permissions with admin consent. Adds
    * one Graph message lookup per group-chat message that yields no media.
    * Default: false.
