From 36cc7bb1059942ec1c7714a689dda577b4bd8398 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 31 Jul 2026 08:03:37 -0700 Subject: [PATCH] fix(imessage): preserve voice intent and provider delivery failures (#116889) * fix(imessage): honor configured attachment send transport * fix(imessage): preserve send outcomes and voice delivery * fix(imessage): preserve native voice transport contracts --------- Co-authored-by: Peter Steinberger --- .../imessage/src/monitor/deliver.test.ts | 22 ++++ extensions/imessage/src/monitor/deliver.ts | 1 + extensions/imessage/src/send.test.ts | 112 ++++++++++++++++++ extensions/imessage/src/send.ts | 41 +++++-- 4 files changed, 164 insertions(+), 12 deletions(-) diff --git a/extensions/imessage/src/monitor/deliver.test.ts b/extensions/imessage/src/monitor/deliver.test.ts index 7939eae37c57..ea993387af63 100644 --- a/extensions/imessage/src/monitor/deliver.test.ts +++ b/extensions/imessage/src/monitor/deliver.test.ts @@ -129,6 +129,28 @@ describe("deliverReplies", () => { ]); }); + it("forwards voice-note payloads to the canonical iMessage media sender", async () => { + await deliverReplies({ + cfg: IMESSAGE_TEST_CFG, + replies: [{ mediaUrl: "https://example.com/voice.caf", audioAsVoice: true }], + target: "chat_id:20", + accountId: "acct-2", + runtime, + maxBytes: 8192, + textLimit: 4000, + }); + + expect(sendMessageIMessageMock).toHaveBeenCalledWith( + "chat_id:20", + "", + expect.objectContaining({ + accountId: "acct-2", + audioAsVoice: true, + mediaUrl: "https://example.com/voice.caf", + }), + ); + }); + it("records durable outbound sends in the sent-message cache", async () => { const remember = vi.fn(); const send = createIMessageEchoCachingSend({ diff --git a/extensions/imessage/src/monitor/deliver.ts b/extensions/imessage/src/monitor/deliver.ts index a12a07713bdc..377cb6978169 100644 --- a/extensions/imessage/src/monitor/deliver.ts +++ b/extensions/imessage/src/monitor/deliver.ts @@ -62,6 +62,7 @@ export async function deliverReplies(params: { const sent = await sendMessageIMessage(target, caption ?? "", { config: params.cfg, mediaUrl, + ...(payload.audioAsVoice ? { audioAsVoice: true } : {}), maxBytes, accountId, replyToId: payload.replyToId, diff --git a/extensions/imessage/src/send.test.ts b/extensions/imessage/src/send.test.ts index ad291a7eb679..71abda17fb61 100644 --- a/extensions/imessage/src/send.test.ts +++ b/extensions/imessage/src/send.test.ts @@ -135,6 +135,25 @@ describe("sendMessageIMessage receipts", () => { expect(result.receipt.sentAt).toBeGreaterThan(0); }); + it("rejects an unsuccessful RPC send instead of acknowledging a delivered message", async () => { + const client = createClient({ success: false, error: "recipient is not registered" }); + + await expect( + sendMessageIMessage("+15551234567", "hello", { + config: IMESSAGE_TEST_CFG, + client, + }), + ).rejects.toThrow("recipient is not registered"); + + expect( + hasPersistedIMessageEcho({ + scope: "default:imessage:+15551234567", + text: "hello", + includePendingText: true, + }), + ).toBe(false); + }); + it("drops reply metadata from text sends when reply actions are disabled", async () => { const client = createClient({ guid: "p:0/imsg-plain" }); @@ -573,6 +592,51 @@ describe("sendMessageIMessage receipts", () => { expect(client["request"]).not.toHaveBeenCalled(); }); + it.each([ + { name: "media", audioAsVoice: false, sendTransport: "bridge" }, + { name: "media", audioAsVoice: false, sendTransport: "applescript" }, + { name: "voice", audioAsVoice: true, sendTransport: "bridge" }, + ] as const)( + "honors the configured $sendTransport transport for $name attachment sends", + async ({ audioAsVoice, sendTransport }) => { + const client = createClient({ message_id: 12345 }); + const runCliJson = vi.fn().mockResolvedValueOnce({ messageId: "p:0/configured-media-guid" }); + const mediaPath = audioAsVoice ? "/tmp/voice.caf" : "/tmp/image.png"; + + await sendMessageIMessage("chat_guid:chat-1", "", { + config: { + channels: { + imessage: { + sendTransport: sendTransport === "bridge" ? "applescript" : "bridge", + accounts: { work: { sendTransport } }, + }, + }, + }, + accountId: "work", + client, + mediaUrl: mediaPath, + audioAsVoice, + resolveAttachmentImpl: async () => ({ + path: mediaPath, + contentType: audioAsVoice ? "audio/x-caf" : "image/png", + }), + runCliJson, + }); + + expect(runCliJson).toHaveBeenCalledWith([ + "send-attachment", + "--chat", + "chat-1", + "--file", + mediaPath, + ...(audioAsVoice ? ["--audio"] : []), + "--transport", + sendTransport === "bridge" ? "dylib" : sendTransport, + ]); + expect(getClientMocks(client).request).not.toHaveBeenCalled(); + }, + ); + it("preserves audioAsVoice media when replying to an iMessage thread", async () => { const client = createClient({ message_id: 12345 }); const runCliJson = vi.fn().mockResolvedValueOnce({ messageId: "p:0/threaded-voice-guid" }); @@ -610,6 +674,54 @@ describe("sendMessageIMessage receipts", () => { expect(client["request"]).not.toHaveBeenCalled(); }); + it.each([undefined, "p:0/reply-guid"])( + "rejects AppleScript voice notes without downgrading native audio (reply: %s)", + async (replyToId) => { + const client = createClient({ guid: "should-not-send" }); + const runCliJson = vi.fn(); + + await expect( + sendMessageIMessage("chat_guid:chat-1", "", { + config: { channels: { imessage: { sendTransport: "applescript" } } }, + client, + conversationReadOrigin: "direct-operator", + mediaUrl: "/tmp/voice.caf", + audioAsVoice: true, + replyToId, + resolveAttachmentImpl: async () => ({ + path: "/tmp/voice.caf", + contentType: "audio/x-caf", + }), + runCliJson, + }), + ).rejects.toThrow("voice messages require bridge transport"); + + expect(runCliJson).not.toHaveBeenCalled(); + expect(getClientMocks(client).request).not.toHaveBeenCalled(); + }, + ); + + it("does not downgrade voice notes when the native attachment bridge is unavailable", async () => { + const client = createClient({ guid: "should-not-send" }); + const runCliJson = vi.fn().mockRejectedValueOnce(new Error("private API bridge unavailable")); + + await expect( + sendMessageIMessage("chat_guid:chat-1", "", { + config: IMESSAGE_TEST_CFG, + client, + mediaUrl: "/tmp/voice.caf", + audioAsVoice: true, + resolveAttachmentImpl: async () => ({ + path: "/tmp/voice.caf", + contentType: "audio/x-caf", + }), + runCliJson, + }), + ).rejects.toThrow("private API bridge unavailable"); + + expect(getClientMocks(client).request).not.toHaveBeenCalled(); + }); + it("drops reply metadata from media sends when reply actions are disabled", async () => { const client = createClient({ message_id: 12345 }); const runCliJson = vi.fn().mockResolvedValueOnce({ messageId: "p:0/plain-media-guid" }); diff --git a/extensions/imessage/src/send.ts b/extensions/imessage/src/send.ts index 6ec66acefa4c..795bae5df386 100644 --- a/extensions/imessage/src/send.ts +++ b/extensions/imessage/src/send.ts @@ -454,7 +454,7 @@ function resolveOutboundEchoScope(params: { return `${params.accountId}:imessage:${params.target.to}`; } -function resolveIMessageCliFailure(result: Record): string | null { +function resolveIMessageSendFailure(result: Record): string | null { if (result.success !== false) { return null; } @@ -560,6 +560,7 @@ async function trySendAttachmentForTarget(params: { dbPath?: string; target: ReturnType; service?: IMessageService; + sendTransport: IMessageSendTransport; filePath: string; audioAsVoice?: boolean; replyToId?: string; @@ -569,6 +570,11 @@ async function trySendAttachmentForTarget(params: { runCliJson: (args: readonly string[]) => Promise>; resolveMessageGuidImpl?: IMessageSendOpts["resolveMessageGuidImpl"]; }): Promise { + if (params.audioAsVoice && params.sendTransport === "applescript") { + throw new Error( + "iMessage voice messages require bridge transport; AppleScript cannot send native voice notes. Set sendTransport to bridge or auto.", + ); + } let attachmentChatTarget: string | null; try { attachmentChatTarget = await resolveAttachmentChatTarget({ @@ -577,12 +583,15 @@ async function trySendAttachmentForTarget(params: { runCliJson: params.runCliJson, }); } catch (error) { - if (isAttachmentCommandFallbackError(error)) { + if (!params.audioAsVoice && isAttachmentCommandFallbackError(error)) { return null; } throw error; } if (!attachmentChatTarget) { + if (params.audioAsVoice) { + throw new Error("iMessage voice messages require an existing chat and bridge transport."); + } return null; } @@ -611,20 +620,21 @@ async function trySendAttachmentForTarget(params: { ...(params.audioAsVoice ? ["--audio"] : []), ...(params.replyToId ? ["--reply-to", params.replyToId] : []), "--transport", - "auto", + // One-shot imsg names its private-API transport dylib; JSON-RPC calls it bridge. + params.sendTransport === "bridge" ? "dylib" : params.sendTransport, ]); } catch (error) { forgetPersistedIMessageEchoKey(pendingEchoKey); - if (isAttachmentCommandFallbackError(error)) { + if (!params.audioAsVoice && isAttachmentCommandFallbackError(error)) { return null; } throw error; } - const failure = resolveIMessageCliFailure(result); + const failure = resolveIMessageSendFailure(result); if (failure) { const error = new Error(failure); forgetPersistedIMessageEchoKey(pendingEchoKey); - if (isAttachmentCommandFallbackError(error)) { + if (!params.audioAsVoice && isAttachmentCommandFallbackError(error)) { return null; } throw error; @@ -789,6 +799,7 @@ export async function sendMessageIMessage( dbPath: chatDbLookupPath, target, service, + sendTransport, filePath, audioAsVoice: opts.audioAsVoice, ...(resolvedReplyToId ? { replyToId: resolvedReplyToId } : {}), @@ -868,6 +879,16 @@ export async function sendMessageIMessage( closedClient = true; await client.stop(); }; + const requestSuccessfulSend = async (sendParams: Record) => { + const response = await client.request>("send", sendParams, { + timeoutMs, + }); + const failure = resolveIMessageSendFailure(response); + if (failure) { + throw new Error(failure); + } + return response; + }; let result: Record; const sendStartedAtMs = Date.now(); let pendingEchoKey: string | undefined; @@ -882,9 +903,7 @@ export async function sendMessageIMessage( pending: true, }); } - result = await client.request>("send", params, { - timeoutMs, - }); + result = await requestSuccessfulSend(params); } catch (error) { if (resolvedReplyToId && isThreadedReplyUnsupportedError(error)) { // #99638: the transport cannot deliver a threaded reply, so resend the @@ -893,9 +912,7 @@ export async function sendMessageIMessage( // reply_to stripped, keeping any file; a further failure propagates. const plainParams = { ...params }; delete plainParams.reply_to; - result = await client.request>("send", plainParams, { - timeoutMs, - }); + result = await requestSuccessfulSend(plainParams); effectiveReplyToId = undefined; } else if (filePath || !isIMessageRpcSendTimeout(error)) { throw error;