fix(outbound): retain attachments when plural media URLs are blank (#128372)

extensions/discord/src/monitor/native-command-reply.test.ts

@@ -155,44 +155,59 @@ describe("deliverDiscordInteractionReply", () => {
     expect(interaction.followUp).not.toHaveBeenCalled();
   });
 
-  it("sends the detected WebP media type across a real interaction multipart request", async () => {
-    const loopback = await createDiscordLoopbackRest();
-    loadWebMediaMock.mockResolvedValue({
-      buffer: Buffer.from("webp"),
-      fileName: "sticker.webp",
-      contentType: "image/webp",
-      kind: "image",
-    });
-    const interaction = {
-      reply: vi.fn(async (data: unknown) =>
-        loopback.rest.post("/interactions/123/token/callback", {
-          body: { type: 4, data },
-        }),
-      ),
-      followUp: vi.fn(),
-    };
-
-    try {
-      await deliverDiscordInteractionReply({
-        interaction: interaction as never,
-        payload: {
-          text: "sticker",
-          mediaUrls: ["file:///tmp/sticker.webp"],
-        },
-        textLimit: 2000,
-        preferFollowUp: false,
-        chunkMode: "length",
+  it.each([
+    {
+      name: "plural media URLs",
+      media: { mediaUrls: ["file:///tmp/sticker.webp"] },
+    },
+    {
+      name: "singular media URL after blank plural URLs",
+      media: { mediaUrls: ["   "], mediaUrl: "file:///tmp/sticker.webp" },
+    },
+  ])(
+    "sends detected WebP media across a real interaction multipart request ($name)",
+    async ({ media }) => {
+      const loopback = await createDiscordLoopbackRest();
+      loadWebMediaMock.mockResolvedValue({
+        buffer: Buffer.from("webp"),
+        fileName: "sticker.webp",
+        contentType: "image/webp",
+        kind: "image",
       });
+      const interaction = {
+        reply: vi.fn(async (data: unknown) =>
+          loopback.rest.post("/interactions/123/token/callback", {
+            body: { type: 4, data },
+          }),
+        ),
+        followUp: vi.fn(),
+      };
 
-      const upload = loopback.requests.find((request) => request.method === "POST");
-      expect(upload?.path).toContain("/interactions/123/token/callback");
-      expect(upload?.contentType).toMatch(/^multipart\/form-data; boundary=/);
-      expect(upload?.body).toContain('name="files[0]"; filename="sticker.webp"');
-      expect(upload?.body).toContain("Content-Type: image/webp");
-    } finally {
-      await loopback.close();
-    }
-  });
+      try {
+        const payload = { text: "sticker", ...media };
+        expect(hasRenderableReplyPayload(payload)).toBe(true);
+
+        await deliverDiscordInteractionReply({
+          interaction: interaction as never,
+          payload,
+          textLimit: 2000,
+          preferFollowUp: false,
+          chunkMode: "length",
+        });
+
+        expect(loadWebMediaMock).toHaveBeenCalledWith("file:///tmp/sticker.webp", {
+          localRoots: undefined,
+        });
+        const upload = loopback.requests.find((request) => request.method === "POST");
+        expect(upload?.path).toContain("/interactions/123/token/callback");
+        expect(upload?.contentType).toMatch(/^multipart\/form-data; boundary=/);
+        expect(upload?.body).toContain('name="files[0]"; filename="sticker.webp"');
+        expect(upload?.body).toContain("Content-Type: image/webp");
+      } finally {
+        await loopback.close();
+      }
+    },
+  );
 });
 
 describe("settleDiscordInteractionWithoutVisibleReply", () => {

extensions/feishu/src/reply-dispatcher.test.ts

@@ -2517,20 +2517,23 @@ describe("createFeishuReplyDispatcher streaming behavior", () => {
     expect(fallbackText).not.toContain(mediaPath);
   });
 
-  it("falls back to legacy mediaUrl when mediaUrls is an empty array", async () => {
-    useNonStreamingAutoAccount();
-    const { options } = createDispatcherHarness();
-    await options.deliver(
-      { text: "caption", mediaUrl: "https://example.com/a.png", mediaUrls: [] },
-      { kind: "final" },
-    );
+  it.each([{ mediaUrls: [] }, { mediaUrls: ["   "] }])(
+    "falls back to legacy mediaUrl when mediaUrls has no usable entries",
+    async ({ mediaUrls }) => {
+      useNonStreamingAutoAccount();
+      const { options } = createDispatcherHarness();
+      await options.deliver(
+        { text: "caption", mediaUrl: "https://example.com/a.png", mediaUrls },
+        { kind: "final" },
+      );
 
-    expect(sendMessageFeishuMock).toHaveBeenCalledTimes(1);
-    expect(sendMediaFeishuMock).toHaveBeenCalledTimes(1);
-    expectMockArgFields(sendMediaFeishuMock, "media send params", {
-      mediaUrl: "https://example.com/a.png",
-    });
-  });
+      expect(sendMessageFeishuMock).toHaveBeenCalledTimes(1);
+      expect(sendMediaFeishuMock).toHaveBeenCalledTimes(1);
+      expectMockArgFields(sendMediaFeishuMock, "media send params", {
+        mediaUrl: "https://example.com/a.png",
+      });
+    },
+  );
 
   it("sends attachments after streaming final markdown replies", async () => {
     const { options } = createDispatcherHarness({

src/infra/outbound/reply-payload-parts.ts

@@ -23,13 +23,10 @@ export function resolveOutboundMediaUrls(payload: {
   mediaUrls?: string[];
   mediaUrl?: string;
 }): string[] {
-  if (payload.mediaUrls?.length) {
+  if (payload.mediaUrls?.some((mediaUrl) => mediaUrl.trim())) {
     return payload.mediaUrls;
   }
-  if (payload.mediaUrl) {
-    return [payload.mediaUrl];
-  }
-  return [];
+  return payload.mediaUrl ? [payload.mediaUrl] : [];
 }
 
 /** Count outbound media items after legacy single-media fallback normalization. */

src/plugin-sdk/reply-payload.test.ts

@@ -600,8 +600,28 @@ describe("resolveOutboundMediaUrls", () => {
       },
       expected: ["https://example.com/legacy.png"],
     },
+    {
+      name: "falls back to the legacy single-media field when plural entries are blank",
+      payload: {
+        mediaUrls: ["   "],
+        mediaUrl: "https://example.com/legacy.png",
+      },
+      expected: ["https://example.com/legacy.png"],
+    },
+    {
+      name: "preserves raw plural entries and duplicates when one attachment is valid",
+      payload: {
+        mediaUrls: ["   ", " https://example.com/a.png ", " https://example.com/a.png "],
+        mediaUrl: "https://example.com/legacy.png",
+      },
+      expected: ["   ", " https://example.com/a.png ", " https://example.com/a.png "],
+    },
   ])("$name", ({ payload, expected }) => {
-    expect(resolveOutboundMediaUrls(payload)).toEqual(expected);
+    const mediaUrls = resolveOutboundMediaUrls(payload);
+    expect(mediaUrls).toEqual(expected);
+    if (payload.mediaUrls?.some((mediaUrl) => mediaUrl.trim())) {
+      expect(mediaUrls).toBe(payload.mediaUrls);
+    }
   });
 });
 
@@ -869,24 +889,26 @@ describe("deliverTextOrMediaReply", () => {
     expect(sendMedia).not.toHaveBeenCalled();
   });
 
-  it("ignores blank media urls before sending", async () => {
+  it.each([
+    {
+      name: "mixed plural entries",
+      payload: { text: "hello", mediaUrls: ["   ", " https://a "] },
+    },
+    {
+      name: "blank plural entries with a valid single attachment",
+      payload: { text: "hello", mediaUrls: ["   "], mediaUrl: " https://a " },
+    },
+  ])("delivers the valid attachment from $name", async ({ payload }) => {
     const sendMedia = vi.fn(async () => undefined);
     const sendText = vi.fn(async () => undefined);
 
     await expect(
-      deliverTextOrMediaReply({
-        payload: { text: "hello", mediaUrls: ["   ", " https://a "] },
-        text: "hello",
-        sendText,
-        sendMedia,
-      }),
+      deliverTextOrMediaReply({ payload, text: "hello", sendText, sendMedia }),
     ).resolves.toBe("media");
 
-    expect(sendMedia).toHaveBeenCalledTimes(1);
-    expect(sendMedia).toHaveBeenCalledWith({
-      mediaUrl: "https://a",
-      caption: "hello",
-    });
+    expect(sendMedia).toHaveBeenCalledOnce();
+    expect(sendMedia).toHaveBeenCalledWith({ mediaUrl: "https://a", caption: "hello" });
+    expect(sendText).not.toHaveBeenCalled();
   });
 });