mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
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 <steipete@macos.shared>
This commit is contained in:
committed by
GitHub
parent
b45ca07ec9
commit
36cc7bb105
@@ -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({
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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" });
|
||||
|
||||
@@ -454,7 +454,7 @@ function resolveOutboundEchoScope(params: {
|
||||
return `${params.accountId}:imessage:${params.target.to}`;
|
||||
}
|
||||
|
||||
function resolveIMessageCliFailure(result: Record<string, unknown>): string | null {
|
||||
function resolveIMessageSendFailure(result: Record<string, unknown>): string | null {
|
||||
if (result.success !== false) {
|
||||
return null;
|
||||
}
|
||||
@@ -560,6 +560,7 @@ async function trySendAttachmentForTarget(params: {
|
||||
dbPath?: string;
|
||||
target: ReturnType<typeof parseIMessageTarget>;
|
||||
service?: IMessageService;
|
||||
sendTransport: IMessageSendTransport;
|
||||
filePath: string;
|
||||
audioAsVoice?: boolean;
|
||||
replyToId?: string;
|
||||
@@ -569,6 +570,11 @@ async function trySendAttachmentForTarget(params: {
|
||||
runCliJson: (args: readonly string[]) => Promise<Record<string, unknown>>;
|
||||
resolveMessageGuidImpl?: IMessageSendOpts["resolveMessageGuidImpl"];
|
||||
}): Promise<IMessageSendResult | null> {
|
||||
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<string, unknown>) => {
|
||||
const response = await client.request<Record<string, unknown>>("send", sendParams, {
|
||||
timeoutMs,
|
||||
});
|
||||
const failure = resolveIMessageSendFailure(response);
|
||||
if (failure) {
|
||||
throw new Error(failure);
|
||||
}
|
||||
return response;
|
||||
};
|
||||
let result: Record<string, unknown>;
|
||||
const sendStartedAtMs = Date.now();
|
||||
let pendingEchoKey: string | undefined;
|
||||
@@ -882,9 +903,7 @@ export async function sendMessageIMessage(
|
||||
pending: true,
|
||||
});
|
||||
}
|
||||
result = await client.request<Record<string, unknown>>("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<Record<string, unknown>>("send", plainParams, {
|
||||
timeoutMs,
|
||||
});
|
||||
result = await requestSuccessfulSend(plainParams);
|
||||
effectiveReplyToId = undefined;
|
||||
} else if (filePath || !isIMessageRpcSendTimeout(error)) {
|
||||
throw error;
|
||||
|
||||
Reference in New Issue
Block a user