fix(imessage): cover all attachment param aliases in reply guard

omarshahine · omarshahine · commit cb83f12023ea · 2026-05-09T07:18:27.000-07:00
Codex review (P2) flagged that the original guard only checked four
param names. The shared message-tool send schema also accepts `path`
(plus agents commonly reach for `fileUrl`, `mediaUrls`), so a reply
that supplied any of those still fell through to `sendRichMessage` and
silently dropped the attachment exactly like the bug being fixed.

Hoist the alias list into a constant, replace the four-key boolean OR
with a `find` over it, and add `hasNonEmptyAttachmentValue` so
array-shaped params (`mediaUrls: [...]`) also trip the guard. Test now
exercises every alias, including the array variant.
diff --git a/extensions/imessage/src/actions.test.ts b/extensions/imessage/src/actions.test.ts
@@ -290,7 +290,17 @@ describe("imessage message actions", () => {
     });
     runtimeMock.resolveChatGuidForTarget.mockResolvedValue("iMessage;+;resolved-ident");
 
-    for (const attachmentField of ["media", "mediaUrl", "filePath", "buffer"]) {
+    const stringValue = "/tmp/anything.png";
+    const cases: Array<{ field: string; value: unknown }> = [
+      { field: "media", value: stringValue },
+      { field: "mediaUrl", value: stringValue },
+      { field: "mediaUrls", value: [stringValue, "/tmp/extra.png"] },
+      { field: "filePath", value: stringValue },
+      { field: "fileUrl", value: stringValue },
+      { field: "path", value: stringValue },
+      { field: "buffer", value: stringValue },
+    ];
+    for (const { field, value } of cases) {
       runtimeMock.sendRichMessage.mockClear();
       await expect(
         imessageMessageActions.handleAction?.({
@@ -300,7 +310,7 @@ describe("imessage message actions", () => {
             chatIdentifier: "team-thread",
             messageId: "message-guid",
             text: "🦞 here it is",
-            [attachmentField]: "/tmp/anything.png",
+            [field]: value,
           },
         } as never),
       ).rejects.toThrow(/iMessage reply does not support attachments/);
diff --git a/extensions/imessage/src/actions.ts b/extensions/imessage/src/actions.ts
@@ -240,6 +240,43 @@ function decodeBase64Buffer(params: Record<string, unknown>, action: string): Ui
   return Uint8Array.from(Buffer.from(base64Buffer, "base64"));
 }
 
+// Param names that any caller would reasonably treat as "attach this file
+// to the message". Covers everything the shared message-tool send schema
+// declares (`media`, `buffer`, `path`, `filePath`) plus the plural/url
+// variants that agents and channel adapters tend to reach for, so the reply
+// guard refuses identically regardless of which alias the caller picked.
+const REPLY_ATTACHMENT_PARAM_NAMES: readonly string[] = [
+  "media",
+  "mediaUrl",
+  "mediaUrls",
+  "filePath",
+  "fileUrl",
+  "path",
+  "buffer",
+] as const;
+
+// Truthy attachment-param check that tolerates string, array, and object
+// shapes — agents commonly pass `media: "/tmp/x.png"` as a string but also
+// `mediaUrls: ["/tmp/x.png"]` as an array, and we want both to trip the
+// reply guard rather than silently falling through.
+function hasNonEmptyAttachmentValue(params: Record<string, unknown>, name: string): boolean {
+  const value = params[name];
+  if (value == null) {
+    return false;
+  }
+  if (typeof value === "string") {
+    return value.trim().length > 0;
+  }
+  if (Array.isArray(value)) {
+    return value.some(
+      (entry) =>
+        (typeof entry === "string" && entry.trim().length > 0) ||
+        (entry != null && typeof entry === "object"),
+    );
+  }
+  return typeof value === "object";
+}
+
 // Whitelist of expressive-send effect IDs the bridge accepts. Restricting
 // to a fixed set lets us return a clear error for typos ("invisible_ink"
 // vs "invisibleink") instead of silently forwarding gibberish to the
@@ -472,13 +509,13 @@ export const imessageMessageActions: ChannelMessageActionAdapter = {
       // does not accept attachments — the only path that handles a file is
       // `imsg send --file`, used by sendAttachment/upload-file. Silently
       // dropping the caller's media looked like success and let agents ship
-      // a "here's your image" reply with no attachment attached.
-      const hasMediaParam =
-        readStringParam(params, "media") != null ||
-        readStringParam(params, "mediaUrl") != null ||
-        readStringParam(params, "filePath") != null ||
-        readStringParam(params, "buffer") != null;
-      if (hasMediaParam) {
+      // a "here's your image" reply with no attachment attached. Cover every
+      // attachment-bearing alias the message-tool send schema exposes plus
+      // common plural/url variants agents reach for.
+      const attachmentParam = REPLY_ATTACHMENT_PARAM_NAMES.find((name) =>
+        hasNonEmptyAttachmentValue(params, name),
+      );
+      if (attachmentParam) {
         throw new Error(
           "iMessage reply does not support attachments. Use action 'upload-file' " +
             "(with filePath/filename) or action 'send' (with media) to deliver the file, " +
