diff --git a/extensions/imessage/src/monitor/deliver.test.ts b/extensions/imessage/src/monitor/deliver.test.ts index 894d1d62328..8574911d383 100644 --- a/extensions/imessage/src/monitor/deliver.test.ts +++ b/extensions/imessage/src/monitor/deliver.test.ts @@ -2,7 +2,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { RuntimeEnv } from "../../../../src/runtime.js"; const sendMessageIMessageMock = vi.hoisted(() => - vi.fn().mockResolvedValue({ messageId: "imsg-1" }), + vi.fn().mockImplementation(async (_to: string, message: string) => ({ + messageId: "imsg-1", + sentText: message, + })), ); const chunkTextWithModeMock = vi.hoisted(() => vi.fn((text: string) => [text])); const resolveChunkModeMock = vi.hoisted(() => vi.fn(() => "length")); @@ -135,9 +138,15 @@ describe("deliverReplies", () => { ); }); - it("records outbound text and message ids in sent-message cache", async () => { + it("records outbound text and message ids in sent-message cache (post-send only)", async () => { + // Fix for #47830: remember() is called ONLY after each chunk is sent, + // never with the full un-chunked text before sending begins. + // Pre-send population widened the false-positive window in self-chat. const remember = vi.fn(); chunkTextWithModeMock.mockImplementation((text: string) => text.split("|")); + sendMessageIMessageMock + .mockResolvedValueOnce({ messageId: "imsg-1", sentText: "first" }) + .mockResolvedValueOnce({ messageId: "imsg-2", sentText: "second" }); await deliverReplies({ replies: [{ text: "first|second" }], @@ -150,14 +159,39 @@ describe("deliverReplies", () => { sentMessageCache: { remember }, }); - expect(remember).toHaveBeenCalledWith("acct-3:chat_id:30", { text: "first|second" }); + // Only the two per-chunk post-send calls — no pre-send full-text call. + expect(remember).toHaveBeenCalledTimes(2); expect(remember).toHaveBeenCalledWith("acct-3:chat_id:30", { text: "first", messageId: "imsg-1", }); expect(remember).toHaveBeenCalledWith("acct-3:chat_id:30", { text: "second", - messageId: "imsg-1", + messageId: "imsg-2", + }); + }); + + it("records the actual sent placeholder for media-only replies", async () => { + const remember = vi.fn(); + sendMessageIMessageMock.mockResolvedValueOnce({ + messageId: "imsg-media-1", + sentText: "", + }); + + await deliverReplies({ + replies: [{ mediaUrls: ["https://example.com/a.jpg"] }], + target: "chat_id:40", + client, + accountId: "acct-4", + runtime, + maxBytes: 2048, + textLimit: 4000, + sentMessageCache: { remember }, + }); + + expect(remember).toHaveBeenCalledWith("acct-4:chat_id:40", { + text: "", + messageId: "imsg-media-1", }); }); }); diff --git a/extensions/imessage/src/monitor/deliver.ts b/extensions/imessage/src/monitor/deliver.ts index 708d319b640..097cb20d259 100644 --- a/extensions/imessage/src/monitor/deliver.ts +++ b/extensions/imessage/src/monitor/deliver.ts @@ -38,9 +38,6 @@ export async function deliverReplies(params: { const reply = resolveSendableOutboundReplyParts(payload, { text: convertMarkdownTables(rawText, tableMode), }); - if (!reply.hasMedia && reply.hasText) { - sentMessageCache?.remember(scope, { text: reply.text }); - } const delivered = await deliverTextOrMediaReply({ payload, text: reply.text, @@ -52,7 +49,11 @@ export async function deliverReplies(params: { accountId, replyToId: payload.replyToId, }); - sentMessageCache?.remember(scope, { text: chunk, messageId: sent.messageId }); + // Post-send cache population (#47830): caching happens after each chunk is sent, + // not before. The window between send completion and cache write is sub-millisecond; + // the next SQLite inbound poll is 1-2s away, so no echo can arrive before the + // cache entry exists. + sentMessageCache?.remember(scope, { text: sent.sentText, messageId: sent.messageId }); }, sendMedia: async ({ mediaUrl, caption }) => { const sent = await sendMessageIMessage(target, caption ?? "", { @@ -63,7 +64,7 @@ export async function deliverReplies(params: { replyToId: payload.replyToId, }); sentMessageCache?.remember(scope, { - text: caption || undefined, + text: sent.sentText || undefined, messageId: sent.messageId, }); }, diff --git a/extensions/imessage/src/monitor/echo-cache.ts b/extensions/imessage/src/monitor/echo-cache.ts index 06f5ee847f5..5104596ff1c 100644 --- a/extensions/imessage/src/monitor/echo-cache.ts +++ b/extensions/imessage/src/monitor/echo-cache.ts @@ -5,12 +5,22 @@ export type SentMessageLookup = { export type SentMessageCache = { remember: (scope: string, lookup: SentMessageLookup) => void; - has: (scope: string, lookup: SentMessageLookup) => boolean; + /** + * Check whether an inbound message matches a recently-sent outbound message. + * + * @param skipIdShortCircuit - When true, skip the early return on message-ID + * mismatch and fall through to text-based matching. Use this for self-chat + * `is_from_me=true` messages where the inbound ID is a numeric SQLite row ID + * that will never match the GUID outbound IDs, but text matching is still + * the right way to identify agent reply echoes. + */ + has: (scope: string, lookup: SentMessageLookup, skipIdShortCircuit?: boolean) => boolean; }; -// Keep the text fallback short so repeated user replies like "ok" are not -// suppressed for long; delayed reflections should match the stronger message-id key. -const SENT_MESSAGE_TEXT_TTL_MS = 5_000; +// Echo arrival observed at ~2.2s on M4 Mac Mini (SQLite poll interval is the bottleneck). +// 4s provides ~80% margin. If echoes arrive after TTL expiry, the system degrades to +// duplicate delivery (noisy but not lossy) — never message loss. +const SENT_MESSAGE_TEXT_TTL_MS = 4_000; const SENT_MESSAGE_ID_TTL_MS = 60_000; function normalizeEchoTextKey(text: string | undefined): string | null { @@ -34,6 +44,7 @@ function normalizeEchoMessageIdKey(messageId: string | undefined): string | null class DefaultSentMessageCache implements SentMessageCache { private textCache = new Map(); + private textBackedByIdCache = new Map(); private messageIdCache = new Map(); remember(scope: string, lookup: SentMessageLookup): void { @@ -44,20 +55,33 @@ class DefaultSentMessageCache implements SentMessageCache { const messageIdKey = normalizeEchoMessageIdKey(lookup.messageId); if (messageIdKey) { this.messageIdCache.set(`${scope}:${messageIdKey}`, Date.now()); + if (textKey) { + this.textBackedByIdCache.set(`${scope}:${textKey}`, Date.now()); + } } this.cleanup(); } - has(scope: string, lookup: SentMessageLookup): boolean { + has(scope: string, lookup: SentMessageLookup, skipIdShortCircuit = false): boolean { this.cleanup(); + const textKey = normalizeEchoTextKey(lookup.text); const messageIdKey = normalizeEchoMessageIdKey(lookup.messageId); if (messageIdKey) { const idTimestamp = this.messageIdCache.get(`${scope}:${messageIdKey}`); if (idTimestamp && Date.now() - idTimestamp <= SENT_MESSAGE_ID_TTL_MS) { return true; } + const textTimestamp = textKey ? this.textCache.get(`${scope}:${textKey}`) : undefined; + const textBackedByIdTimestamp = textKey + ? this.textBackedByIdCache.get(`${scope}:${textKey}`) + : undefined; + const hasTextOnlyMatch = + typeof textTimestamp === "number" && + (!textBackedByIdTimestamp || textTimestamp > textBackedByIdTimestamp); + if (!skipIdShortCircuit && !hasTextOnlyMatch) { + return false; + } } - const textKey = normalizeEchoTextKey(lookup.text); if (textKey) { const textTimestamp = this.textCache.get(`${scope}:${textKey}`); if (textTimestamp && Date.now() - textTimestamp <= SENT_MESSAGE_TEXT_TTL_MS) { @@ -74,6 +98,11 @@ class DefaultSentMessageCache implements SentMessageCache { this.textCache.delete(key); } } + for (const [key, timestamp] of this.textBackedByIdCache.entries()) { + if (now - timestamp > SENT_MESSAGE_TEXT_TTL_MS) { + this.textBackedByIdCache.delete(key); + } + } for (const [key, timestamp] of this.messageIdCache.entries()) { if (now - timestamp > SENT_MESSAGE_ID_TTL_MS) { this.messageIdCache.delete(key); diff --git a/extensions/imessage/src/monitor/inbound-processing.test.ts b/extensions/imessage/src/monitor/inbound-processing.test.ts index 4575a28de36..3e61490f389 100644 --- a/extensions/imessage/src/monitor/inbound-processing.test.ts +++ b/extensions/imessage/src/monitor/inbound-processing.test.ts @@ -84,6 +84,31 @@ describe("resolveIMessageInboundDecision echo detection", () => { ); }); + it("matches attachment-only echoes by bodyText placeholder", () => { + const echoHas = vi.fn((_scope: string, lookup: { text?: string; messageId?: string }) => { + return lookup.text === "" && lookup.messageId === "42"; + }); + + const decision = resolveDecision({ + message: { + id: 42, + text: "", + }, + messageText: "", + bodyText: "", + echoCache: { has: echoHas }, + }); + + expect(decision).toEqual({ kind: "drop", reason: "echo" }); + expect(echoHas).toHaveBeenCalledWith( + "default:imessage:+15555550123", + expect.objectContaining({ + text: "", + messageId: "42", + }), + ); + }); + it("drops reflected self-chat duplicates after seeing the from-me copy", () => { const selfChatCache = createSelfChatCache(); const createdAt = "2026-03-02T20:58:10.649Z"; diff --git a/extensions/imessage/src/monitor/inbound-processing.ts b/extensions/imessage/src/monitor/inbound-processing.ts index 4f87be6a930..be230fa70b8 100644 --- a/extensions/imessage/src/monitor/inbound-processing.ts +++ b/extensions/imessage/src/monitor/inbound-processing.ts @@ -104,7 +104,13 @@ export function resolveIMessageInboundDecision(params: { storeAllowFrom: string[]; historyLimit: number; groupHistories: Map; - echoCache?: { has: (scope: string, lookup: { text?: string; messageId?: string }) => boolean }; + echoCache?: { + has: ( + scope: string, + lookup: { text?: string; messageId?: string }, + skipIdShortCircuit?: boolean, + ) => boolean; + }; selfChatCache?: SelfChatCache; logVerbose?: (msg: string) => void; }): IMessageInboundDecision { @@ -118,6 +124,8 @@ export function resolveIMessageInboundDecision(params: { const chatGuid = params.message.chat_guid ?? undefined; const chatIdentifier = params.message.chat_identifier ?? undefined; const createdAt = params.message.created_at ? Date.parse(params.message.created_at) : undefined; + const messageText = params.messageText.trim(); + const bodyText = params.bodyText.trim(); const groupIdCandidate = chatId !== undefined ? String(chatId) : undefined; const groupListPolicy = groupIdCandidate @@ -145,12 +153,61 @@ export function resolveIMessageInboundDecision(params: { isGroup, chatId, sender, - text: params.bodyText, + text: bodyText, createdAt, }; + // Self-chat detection: in self-chat, sender == chat_identifier (both are the + // user's own handle). When is_from_me=true in self-chat, the message could be + // either: (a) a real user message typed by the user, or (b) an agent reply + // echo reflected back by iMessage. We must distinguish them. + const isSelfChat = + !isGroup && + chatIdentifier != null && + normalizeIMessageHandle(sender) === normalizeIMessageHandle(chatIdentifier); + // Track whether we already processed the is_from_me=true self-chat path. + // When true, the selfChatCache.has() check below must be skipped — we just + // called remember() and would immediately match our own entry. + let skipSelfChatHasCheck = false; + const inboundMessageId = + normalizeReplyField(params.message.guid) ?? + (params.message.id != null ? String(params.message.id) : undefined); + if (params.message.is_from_me) { + // Always cache in selfChatCache so the upcoming is_from_me=false reflection + // (which arrives 2-3s later) is correctly identified and dropped. params.selfChatCache?.remember(selfChatLookup); - return { kind: "drop", reason: "from me" }; + + if (isSelfChat) { + // In self-chat, is_from_me=true could be a real user message OR an agent + // reply echo. Use the echo cache with skipIdShortCircuit=true to check + // whether this text matches a recently-sent agent reply. + const echoScope = buildIMessageEchoScope({ + accountId: params.accountId, + isGroup, + chatId, + sender, + }); + if ( + params.echoCache && + (bodyText || inboundMessageId) && + params.echoCache.has( + echoScope, + { text: bodyText || undefined, messageId: inboundMessageId }, + !normalizeReplyField(params.message.guid), + ) + ) { + return { kind: "drop", reason: "agent echo in self-chat" }; + } + // Echo cache missed → this is a real user message in self-chat. Process it. + // Skip the selfChatCache.has() check below — we just remember()d ourselves + // and would immediately match our own entry. + skipSelfChatHasCheck = true; + // Fall through to rest of decision logic (access control, etc.) + } else { + // Normal DM or group: is_from_me=true means this is an outbound message + // notification that we sent. Drop it. + return { kind: "drop", reason: "from me" }; + } } if (isGroup && !chatId) { return { kind: "drop", reason: "group without chat_id" }; @@ -222,18 +279,17 @@ export function resolveIMessageInboundDecision(params: { chatId, }); const mentionRegexes = buildMentionRegexes(params.cfg, route.agentId); - const messageText = params.messageText.trim(); - const bodyText = params.bodyText.trim(); if (!bodyText) { return { kind: "drop", reason: "empty body" }; } - if ( - params.selfChatCache?.has({ - ...selfChatLookup, - text: bodyText, - }) - ) { + const selfChatHit = skipSelfChatHasCheck + ? false + : params.selfChatCache?.has({ + ...selfChatLookup, + text: bodyText, + }); + if (selfChatHit) { const preview = sanitizeTerminalText(truncateUtf16Safe(bodyText, 50)); params.logVerbose?.(`imessage: dropping self-chat reflected duplicate: "${preview}"`); return { kind: "drop", reason: "self-chat echo" }; @@ -241,7 +297,6 @@ export function resolveIMessageInboundDecision(params: { // Echo detection: check if the received message matches a recently sent message. // Scope by conversation so same text in different chats is not conflated. - const inboundMessageId = params.message.id != null ? String(params.message.id) : undefined; if (params.echoCache && (messageText || inboundMessageId)) { const echoScope = buildIMessageEchoScope({ accountId: params.accountId, @@ -251,12 +306,12 @@ export function resolveIMessageInboundDecision(params: { }); if ( params.echoCache.has(echoScope, { - text: messageText || undefined, + text: bodyText || undefined, messageId: inboundMessageId, }) ) { params.logVerbose?.( - describeIMessageEchoDropLog({ messageText, messageId: inboundMessageId }), + describeIMessageEchoDropLog({ messageText: bodyText, messageId: inboundMessageId }), ); return { kind: "drop", reason: "echo" }; } diff --git a/extensions/imessage/src/monitor/parse-notification.ts b/extensions/imessage/src/monitor/parse-notification.ts index 98ad941665c..7401009e48b 100644 --- a/extensions/imessage/src/monitor/parse-notification.ts +++ b/extensions/imessage/src/monitor/parse-notification.ts @@ -61,6 +61,7 @@ export function parseIMessageNotification(raw: unknown): IMessagePayload | null const message: IMessagePayload = maybeMessage; if ( !isOptionalNumber(message.id) || + !isOptionalString(message.guid) || !isOptionalNumber(message.chat_id) || !isOptionalString(message.sender) || !isOptionalBoolean(message.is_from_me) || diff --git a/extensions/imessage/src/monitor/self-chat-dedupe.test.ts b/extensions/imessage/src/monitor/self-chat-dedupe.test.ts new file mode 100644 index 00000000000..d6f6e900371 --- /dev/null +++ b/extensions/imessage/src/monitor/self-chat-dedupe.test.ts @@ -0,0 +1,624 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../../../src/config/config.js"; +import { createSentMessageCache } from "./echo-cache.js"; +import { resolveIMessageInboundDecision } from "./inbound-processing.js"; +import { createSelfChatCache } from "./self-chat-cache.js"; + +/** + * Self-chat dedupe regression tests for #47830. + * + * PR #38440 introduced a SentMessageCache to suppress echo messages when the + * agent replies in iMessage. In self-chat (user messaging themselves), the + * sender == target so the echo scope collides, causing legitimate user + * messages to be silently dropped when text happens to match recent agent + * output. + * + * These tests verify: + * 1. User messages in self-chat are NOT dropped (even if text matches agent output) + * 2. Genuine agent echo reflections ARE still dropped + * 3. Different-text messages pass through unaffected + * 4. Chunked replies don't cause false drops of user messages matching a chunk + */ + +type InboundDecisionParams = Parameters[0]; + +const cfg = {} as OpenClawConfig; + +function createParams( + overrides: Omit, "message"> & { + message?: Partial; + } = {}, +): InboundDecisionParams { + const { message: msgOverrides, ...restOverrides } = overrides; + const message = { + id: 100, + sender: "+15551234567", + text: "Hello", + is_from_me: false, + is_group: false, + ...msgOverrides, + }; + const messageText = restOverrides.messageText ?? message.text ?? ""; + const bodyText = restOverrides.bodyText ?? messageText; + return { + cfg, + accountId: "default", + opts: undefined, + allowFrom: [], + groupAllowFrom: [], + groupPolicy: "open", + dmPolicy: "open", + storeAllowFrom: [], + historyLimit: 0, + groupHistories: new Map(), + echoCache: undefined, + selfChatCache: undefined, + logVerbose: undefined, + ...restOverrides, + message, + messageText, + bodyText, + }; +} + +describe("echo cache — message ID type canary (#47830)", () => { + // Tests the implicit contract that outbound GUIDs (e.g. "p:0/abc-def-123") + // never match inbound SQLite row IDs (e.g. "200"). If iMessage ever changes + // ID schemes, this test should break loudly. + it("outbound GUID format and inbound SQLite row ID format never collide", () => { + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + // Outbound messageId is a GUID format string + echoCache.remember(scope, { text: "test", messageId: "p:0/abc-def-123" }); + + // An inbound SQLite row ID (numeric string) should NOT match the GUID + expect(echoCache.has(scope, { text: "different", messageId: "200" })).toBe(false); + + // The original GUID should still match + expect(echoCache.has(scope, { text: "different", messageId: "p:0/abc-def-123" })).toBe(true); + }); + + it('falls back to text when outbound messageId was junk ("ok")', () => { + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + // "ok" is normalized out and should not populate the ID cache. + echoCache.remember(scope, { text: "text-only fallback", messageId: "ok" }); + + // Inbound has a numeric SQLite ID that does not exist in cache. Since this + // scope has no real cached IDs, has() must still fall through to text match. + expect(echoCache.has(scope, { text: "text-only fallback", messageId: "200" })).toBe(true); + }); + + it("keeps ID short-circuit when scope has real outbound GUID IDs", () => { + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + echoCache.remember(scope, { text: "guid-backed", messageId: "p:0/abc-def-123" }); + + // Different inbound numeric ID should still short-circuit to false. + expect(echoCache.has(scope, { text: "guid-backed", messageId: "200" })).toBe(false); + }); +}); + +describe("echo cache — backward compat for channels without messageId", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + // Proves text-fallback echo detection still works when no messageId is present + // on either side. Critical for backward compat with channels that don't + // populate messageId. + it("text-only remember/has works within TTL", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + echoCache.remember(scope, { text: "no id message" }); + + vi.advanceTimersByTime(2000); + expect(echoCache.has(scope, { text: "no id message" })).toBe(true); + }); + + it("text-only has returns false after TTL expiry", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + echoCache.remember(scope, { text: "no id message" }); + + vi.advanceTimersByTime(5000); + expect(echoCache.has(scope, { text: "no id message" })).toBe(false); + }); + + it("text-only has returns false for different text", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + echoCache.remember(scope, { text: "no id message" }); + + vi.advanceTimersByTime(1000); + expect(echoCache.has(scope, { text: "totally different text" })).toBe(false); + }); +}); + +describe("self-chat dedupe — #47830", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("does NOT drop a user message that matches recently-sent agent text (self-chat scope collision)", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const selfChatCache = createSelfChatCache(); + + // Agent sends "Hello" to self-chat target +15551234567 + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "Hello", messageId: "agent-msg-1" }); + + // 2 seconds later, user sends "Hello" to themselves (different message id) + vi.advanceTimersByTime(2000); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 200, + sender: "+15551234567", + text: "Hello", + is_from_me: false, + }, + messageText: "Hello", + bodyText: "Hello", + echoCache, + selfChatCache, + }), + ); + + // BUG: Before fix, this was "drop" reason "echo" — user message silently lost. + // After fix: message-id mismatch means this is NOT an echo. + // The echo cache should only match when message IDs match OR when text + // matches and no message ID is available on inbound. + expect(decision.kind).toBe("dispatch"); + }); + + it("DOES drop genuine agent echo (same message id reflected back)", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + + // Agent sends "Hello" to target + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "Hello", messageId: "agent-msg-1" }); + + // 1 second later, iMessage reflects it back with same message id + vi.advanceTimersByTime(1000); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: "agent-msg-1" as unknown as number, + sender: "+15551234567", + text: "Hello", + is_from_me: false, + }, + messageText: "Hello", + bodyText: "Hello", + echoCache, + }), + ); + + expect(decision).toEqual({ kind: "drop", reason: "echo" }); + }); + + it("does NOT drop different-text messages even within TTL", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + + // Agent sends "Hello" + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "Hello", messageId: "agent-msg-1" }); + + vi.advanceTimersByTime(1000); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 201, + sender: "+15551234567", + text: "Goodbye", + is_from_me: false, + }, + messageText: "Goodbye", + bodyText: "Goodbye", + echoCache, + }), + ); + + expect(decision.kind).toBe("dispatch"); + }); + + it("does NOT drop user messages that match a chunk of a multi-chunk agent reply", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15551234567"; + + // Agent sends a multi-chunk reply: "Part one", "Part two", "Part three" + echoCache.remember(scope, { text: "Part one", messageId: "agent-chunk-1" }); + echoCache.remember(scope, { text: "Part two", messageId: "agent-chunk-2" }); + echoCache.remember(scope, { text: "Part three", messageId: "agent-chunk-3" }); + + vi.advanceTimersByTime(2000); + + // User sends "Part two" (matches chunk 2 text, but different message id) + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 300, + sender: "+15551234567", + text: "Part two", + is_from_me: false, + }, + messageText: "Part two", + bodyText: "Part two", + echoCache, + }), + ); + + // Should NOT be dropped — different message id means not an echo + expect(decision.kind).toBe("dispatch"); + }); + + it("drops echo after text TTL expiry (4s TTL: expired at 5s)", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + // Agent sends text (no message id available) + echoCache.remember(scope, { text: "Hello there" }); + + // After 5 seconds — beyond the 4s TTL, should NOT match + vi.advanceTimersByTime(5000); + + const result = echoCache.has(scope, { text: "Hello there" }); + expect(result).toBe(false); + }); + + // Safe failure mode: TTL expiry causes duplicate delivery (noisy), never message loss (lossy) + it("does NOT catch echo after TTL expiry — safe failure mode is duplicate delivery", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15551234567"; + + // Agent sends "Delayed echo test" + echoCache.remember(scope, { text: "Delayed echo test", messageId: "agent-msg-delayed" }); + + // 4.5 seconds later — beyond 4s TTL + vi.advanceTimersByTime(4500); + + // Echo arrives with no messageId (text-only fallback path) + const result = echoCache.has(scope, { text: "Delayed echo test" }); + + // TTL expired → not caught → duplicate delivery (noisy but safe, not lossy) + expect(result).toBe(false); + }); + + it("still drops text echo within 4s TTL window", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + echoCache.remember(scope, { text: "Hello there" }); + + // After 3 seconds — within the 4s TTL, should still match + vi.advanceTimersByTime(3000); + + const result = echoCache.has(scope, { text: "Hello there" }); + expect(result).toBe(true); + }); +}); + +describe("self-chat is_from_me=true handling (Bruce Phase 2 fix)", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("processes real user self-chat message (is_from_me=true, no echo cache match)", () => { + // User sends "Hello" to themselves — is_from_me=true, sender==chat_identifier + const echoCache = createSentMessageCache(); + const selfChatCache = createSelfChatCache(); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 123703, + sender: "+15551234567", + chat_identifier: "+15551234567", + text: "Hello this is a test message", + is_from_me: true, + is_group: false, + }, + messageText: "Hello this is a test message", + bodyText: "Hello this is a test message", + echoCache, + selfChatCache, + }), + ); + + // Real user message — should be dispatched, not dropped + expect(decision.kind).toBe("dispatch"); + }); + + it("drops agent reply echo in self-chat (is_from_me=true, echo cache text match)", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const selfChatCache = createSelfChatCache(); + + // Agent sends "Hi there!" to self-chat + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "Hi there!", messageId: "p:0/GUID-abc-def" }); + + // 1 second later, iMessage delivers the agent reply as is_from_me=true + // with a SQLite row ID (never matches the GUID) + vi.advanceTimersByTime(1000); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 123706, + guid: "p:0/GUID-abc-def", + sender: "+15551234567", + chat_identifier: "+15551234567", + text: "Hi there!", + is_from_me: true, + is_group: false, + }, + messageText: "Hi there!", + bodyText: "Hi there!", + echoCache, + selfChatCache, + }), + ); + + // Agent echo — should be dropped + expect(decision).toEqual({ kind: "drop", reason: "agent echo in self-chat" }); + }); + + it("drops attachment-only agent echo in self-chat via bodyText placeholder", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const selfChatCache = createSelfChatCache(); + + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "", messageId: "p:0/GUID-media" }); + + vi.advanceTimersByTime(1000); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 123707, + guid: "p:0/GUID-media", + sender: "+15551234567", + chat_identifier: "+15551234567", + text: "", + is_from_me: true, + is_group: false, + }, + messageText: "", + bodyText: "", + echoCache, + selfChatCache, + }), + ); + + expect(decision).toEqual({ kind: "drop", reason: "agent echo in self-chat" }); + }); + + it("does not drop a real self-chat image just because a recent agent image used the same placeholder", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const selfChatCache = createSelfChatCache(); + + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "", messageId: "p:0/GUID-agent-image" }); + + vi.advanceTimersByTime(1000); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 123708, + guid: "p:0/GUID-user-image", + sender: "+15551234567", + chat_identifier: "+15551234567", + text: "", + is_from_me: true, + is_group: false, + }, + messageText: "", + bodyText: "", + echoCache, + selfChatCache, + }), + ); + + expect(decision.kind).toBe("dispatch"); + }); + + it("drops is_from_me=false reflection via selfChatCache (existing behavior preserved)", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const selfChatCache = createSelfChatCache(); + const createdAt = "2026-03-24T12:00:00.000Z"; + + // Step 1: is_from_me=true copy arrives (real user message) → processed, selfChatCache populated + const first = resolveIMessageInboundDecision( + createParams({ + message: { + id: 123703, + sender: "+15551234567", + chat_identifier: "+15551234567", + text: "Hello", + created_at: createdAt, + is_from_me: true, + is_group: false, + }, + messageText: "Hello", + bodyText: "Hello", + selfChatCache, + }), + ); + expect(first.kind).toBe("dispatch"); + + // Step 2: is_from_me=false reflection arrives 2s later with same text+createdAt + vi.advanceTimersByTime(2200); + const second = resolveIMessageInboundDecision( + createParams({ + message: { + id: 123704, + sender: "+15551234567", + chat_identifier: "+15551234567", + text: "Hello", + created_at: createdAt, + is_from_me: false, + is_group: false, + }, + messageText: "Hello", + bodyText: "Hello", + selfChatCache, + }), + ); + // Reflection correctly dropped + expect(second).toEqual({ kind: "drop", reason: "self-chat echo" }); + }); + + it("normal DM is_from_me=true is still dropped (regression test)", () => { + const selfChatCache = createSelfChatCache(); + + // Normal DM with is_from_me=true: in iMessage, sender is the local user's + // handle and chat_identifier is the OTHER person's handle. They differ, + // so this is NOT self-chat. + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: 9999, + sender: "+15551234567", // local user sent this + chat_identifier: "+15555550123", // sent TO this other person + text: "Hello", + is_from_me: true, + is_group: false, + }, + messageText: "Hello", + bodyText: "Hello", + selfChatCache, + }), + ); + + // sender != chat_identifier → not self-chat → dropped as "from me" + expect(decision).toEqual({ kind: "drop", reason: "from me" }); + }); + + it("echo cache text matching works with skipIdShortCircuit=true", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "Cached reply", messageId: "p:0/some-guid" }); + + vi.advanceTimersByTime(1000); + + // Text matches but ID is a SQLite row (format mismatch). With skipIdShortCircuit=true, + // text matching should still fire. + expect(echoCache.has(scope, { text: "Cached reply", messageId: "123799" }, true)).toBe(true); + + // With skipIdShortCircuit=false (default), ID mismatch causes early return false. + expect(echoCache.has(scope, { text: "Cached reply", messageId: "123799" }, false)).toBe(false); + }); +}); + +describe("echo cache — text fallback for null-id inbound messages", () => { + it("still identifies echo via text when inbound message has id: null", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-03-24T12:00:00Z")); + + const echoCache = createSentMessageCache(); + const selfChatCache = createSelfChatCache(); + + // Agent sends "Sounds good" — no messageId available (edge case) + const scope = "default:imessage:+15551234567"; + echoCache.remember(scope, { text: "Sounds good" }); + + // 1 second later, inbound reflection arrives with id: null + vi.advanceTimersByTime(1000); + + const decision = resolveIMessageInboundDecision( + createParams({ + message: { + id: null as unknown as number, + sender: "+15551234567", + text: "Sounds good", + is_from_me: false, + }, + messageText: "Sounds good", + bodyText: "Sounds good", + echoCache, + selfChatCache, + }), + ); + + // With id: null, the text-based fallback path is still active and should + // correctly identify this as an echo. + expect(decision).toEqual({ kind: "drop", reason: "echo" }); + }); +}); + +describe("echo cache — mixed GUID and text-only scopes", () => { + it("still falls back to text for the latest text-only send in a scope with older GUID-backed sends", () => { + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + echoCache.remember(scope, { text: "older guid-backed", messageId: "p:0/GUID-older" }); + echoCache.remember(scope, { text: "latest text-only", messageId: "unknown" }); + + expect(echoCache.has(scope, { text: "latest text-only", messageId: "200" })).toBe(true); + }); + + it("still short-circuits when the latest copy of a text was GUID-backed", () => { + const echoCache = createSentMessageCache(); + const scope = "default:imessage:+15555550123"; + + echoCache.remember(scope, { text: "same text", messageId: "unknown" }); + echoCache.remember(scope, { text: "same text", messageId: "p:0/GUID-newer" }); + + expect(echoCache.has(scope, { text: "same text", messageId: "200" })).toBe(false); + }); +}); diff --git a/extensions/imessage/src/monitor/types.ts b/extensions/imessage/src/monitor/types.ts index a03ed5faea8..db357f56b60 100644 --- a/extensions/imessage/src/monitor/types.ts +++ b/extensions/imessage/src/monitor/types.ts @@ -9,6 +9,7 @@ export type IMessageAttachment = { export type IMessagePayload = { id?: number | null; + guid?: string | null; chat_id?: number | null; sender?: string | null; is_from_me?: boolean | null; diff --git a/extensions/imessage/src/send.ts b/extensions/imessage/src/send.ts index b5ba40c3c54..eb3820b0764 100644 --- a/extensions/imessage/src/send.ts +++ b/extensions/imessage/src/send.ts @@ -33,6 +33,7 @@ export type IMessageSendOpts = { export type IMessageSendResult = { messageId: string; + sentText: string; }; const MAX_REPLY_TO_ID_LENGTH = 256; @@ -78,6 +79,17 @@ function resolveMessageId(result: Record | null | undefined): s return raw ? String(raw).trim() : null; } +function resolveDeliveredIMessageText(text: string, mediaContentType?: string): string { + if (text.trim()) { + return text; + } + const kind = kindFromMime(mediaContentType ?? undefined); + if (!kind) { + return text; + } + return kind === "image" ? "" : ``; +} + export async function sendMessageIMessage( to: string, text: string, @@ -113,12 +125,7 @@ export async function sendMessageIMessage( localRoots: opts.mediaLocalRoots, }); filePath = resolved.path; - if (!message.trim()) { - const kind = kindFromMime(resolved.contentType ?? undefined); - if (kind) { - message = kind === "image" ? "" : ``; - } - } + message = resolveDeliveredIMessageText(message, resolved.contentType ?? undefined); } if (!message.trim() && !filePath) { @@ -172,6 +179,7 @@ export async function sendMessageIMessage( const resolvedId = resolveMessageId(result); return { messageId: resolvedId ?? (result?.ok ? "ok" : "unknown"), + sentText: message, }; } finally { if (shouldClose) {