fix(imessage): require destination_caller_id for self-chat classification (#63980) by neeravmakwana · Pull Request #63989 · openclaw/openclaw

neeravmakwana · 2026-04-10T00:39:03Z

Summary

Problem: In some DM SQLite rows, sender and chat_identifier match for both directions. When destination_caller_id was missing, the monitor treated the thread as self-chat (matchesSelfChatDestination was true because a missing destination was treated as a match). Outbound is_from_me messages then skipped the normal from me drop and could loop when echo cache missed.
Why it matters: Reported as outbound replies echoing back as inbound in DMs (#63980).
What changed: Self-chat classification now requires destination_caller_id to be present and normalized-equal to sender. Tests that model true self-chat include that field; new regression tests cover absent/blank destination with sender === chat_identifier.
What did NOT change: Group handling, echo cache TTL, and the existing path where destination_caller_id differs from sender (normal DM) remain as before.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes iMessage echo detection: DM misidentified as self-chat when sender == chat_identifier #63980
Related fix(imessage): avoid DM self-chat false positives #61619
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: matchesSelfChatDestination used destinationCallerIdNormalized == null || destinationCallerIdNormalized === senderNormalized, so a missing destination behaved like a confirmed self-chat destination match whenever sender === chat_identifier.
Missing detection / guardrail: Ambiguous DM rows without destination_caller_id were indistinguishable from self-chat on those two fields alone.
Contributing context (if known): chat.db row shapes vary; some DMs omit destination in the payload OpenClaw sees.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: extensions/imessage/src/monitor/self-chat-dedupe.test.ts
Scenario the test should lock in: is_from_me + sender === chat_identifier + missing or blank destination_caller_id drops as from me; true self-chat still dispatches when destination_caller_id matches sender.
Why this is the smallest reliable guardrail: Asserts the classification gate directly without requiring live chat.db.
Existing test that already covers this (if any): Partial overlap with uses destination_caller_id to avoid DM self-chat false positives (populated destination differing from sender).
If no new test is added, why not: N/A — new cases added.

User-visible / Behavior Changes

iMessage: DMs whose monitor payload omits destination_caller_id are no longer classified as self-chat solely from sender === chat_identifier. True self-chat rows should include destination_caller_id matching the sender (as in existing supported cases).

Diagram (if applicable)

N/A

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS (issue context)
Runtime/container: N/A
Model/provider: N/A
Integration/channel (if any): iMessage (bundled extension)
Relevant config (redacted): N/A

Steps

pnpm test:extension imessage
pnpm check

Expected

All iMessage extension tests pass; lint/typecheck pass.

Actual

Ran locally: iMessage extension tests and pnpm check green.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

Verified scenarios: Unit tests for inbound decision; logic review against #63980.
Edge cases checked: Blank vs absent destination_caller_id; existing DM tests with differing destination_caller_id; self-chat fixtures updated with explicit destination matching sender.
What I did not verify: Live chat.db / imsg on hardware.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Risks and Mitigations

Risk: A true self-chat row that never populates destination_caller_id could be treated as non-self-chat; outbound is_from_me would drop as from me instead of taking the self-chat branch.
- Mitigation: Aligns with using explicit destination metadata when available; self-chat tests use populated destination_caller_id matching sender.

Made with Cursor

greptile-apps · 2026-04-10T00:40:43Z

Greptile Summary

This PR fixes a self-chat misclassification in the iMessage monitor: previously, a missing destination_caller_id caused matchesSelfChatDestination to be true (via the == null branch), so DM outbound rows where sender === chat_identifier were incorrectly routed through the self-chat echo path and could loop. The fix tightens the predicate to require an explicit, non-null destination that matches the sender before treating a message as self-chat.

The one-line logic inversion (== null || → != null &&) is correct and well-scoped. Existing self-chat dispatch tests are updated to supply destination_caller_id, the renamed blank-destination test expectation is flipped to \"from me\", and a new regression test covers the absent-destination case from #63980.

Confidence Score: 5/5

Safe to merge — targeted one-line predicate fix with complete test coverage and no regressions.

All findings are at most P2. The logic inversion correctly fixes the null-treated-as-match bug. Existing self-chat dispatch tests are updated with explicit destination_caller_id, the renamed test expectation matches the new behavior, and a new regression test locks in the absent-destination drop path. The known trade-off (true self-chat rows without destination_caller_id now drop as from me) is explicitly acknowledged in the PR risk section and is the correct conservative default.

No files require special attention.

_{Reviews (1): Last reviewed commit: "fix(imessage): require destination_calle..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54386603dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

neeravmakwana · 2026-04-10T00:51:44Z

Follow-up: AI review (Codex P1) + test alignment

Codex concern (P1 — self-chat message loss when `destination_caller_id` is missing)

Root cause of the interaction: Under the strict self-chat predicate (destination_caller_id must normalize-equal sender), an outbound is_from_me=true row that is not classified as self-chat takes the "from me" drop path. The previous code still called selfChatCache.remember() for every is_from_me=true message before branching. That meant the later is_from_me=false reflection could be dropped as "self-chat echo" even though we never intended the self-chat pipeline — producing silent message loss for row shapes that omit destination_caller_id but might still be “notes to self” in some environments.

Fix: selfChatCache.remember() now runs only inside the confirmed isSelfChat branch (same place as the self-chat echo-cache logic). Normal DMs and ambiguous rows that fail the self-chat gate no longer populate selfChatCache on the "from me" return path.

Net behavior:

iMessage echo detection: DM misidentified as self-chat when sender == chat_identifier #63980 / ambiguous DM: sender === chat_identifier without a usable destination → not self-chat → outbound dropped as "from me" with no cache entry → inbound reflection is not incorrectly suppressed as "self-chat echo" (new regression test: ambiguous DM without destination_caller_id does not lose inbound to self-chat echo).
True self-chat (supported case): rows that include destination_caller_id matching sender still remember() on the outbound leg and still drop the reflected duplicate as before.

Greptile

No code changes required beyond the above; the one-line predicate fix remains as reviewed.

Tests

inbound-processing.test.ts: the “reflected duplicate” scenario now uses explicit chat_identifier + destination_caller_id so it exercises real self-chat: first leg dispatch, second leg "self-chat echo" (the old fixture had no chat_identifier, so it was not self-chat and relied on the unconditional remember() behavior).
self-chat-dedupe.test.ts: self-chat cases that need the self-chat path now set destination_caller_id where missing; blank destination expects "from me" for outbound is_from_me.

Verification

pnpm test:extension imessage and pnpm check are green locally.

If this branch was already pushed without this follow-up, cherry-pick or merge the latest commit that contains the remember() scope change + test updates.

aisle-research-bot · 2026-04-10T08:03:01Z

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

#	Severity	Title
1	🟡 Medium	Inbound iMessage reflection may be dispatched when destination_caller_id is missing, enabling self-echo loops

1. 🟡 Inbound iMessage reflection may be dispatched when destination_caller_id is missing, enabling self-echo loops

Property	Value
Severity	Medium
CWE	CWE-841
Location	`extensions/imessage/src/monitor/inbound-processing.ts:207-335`

Description

resolveIMessageInboundDecision no longer records all is_from_me=true messages in selfChatCache, and it now requires destination_caller_id to be present to classify a conversation as self-chat.

As a result:

For some iMessage SQLite row shapes where sender === chat_identifier but destination_caller_id is absent/blank (documented as ambiguous in-code), outbound messages (is_from_me=true) are dropped as expected.
However, the subsequent reflected row for the same outbound message (is_from_me=false) will not match selfChatCache.has() (because nothing was remembered), so it can proceed to dispatch.
If the outbound message was generated by the agent and the separate echoCache does not contain/retain the sent entry (restart, eviction, missing id/text correlation, etc.), this reflected message can be treated as a fresh inbound user message.

This creates a self-echo / message-loop risk where the agent processes its own outputs as inputs, potentially triggering unintended command execution or recursive reply amplification.

Vulnerable logic:

// destination_caller_id must be present to be considered self-chat
const matchesSelfChatDestination =
  destinationCallerIdNormalized != null && destinationCallerIdNormalized === senderNormalized;
...
if (params.message.is_from_me) {
  if (isSelfChat) {
    params.selfChatCache?.remember(selfChatLookup);
    ...
  } else {
    return { kind: "drop", reason: "from me" };
  }
}
...
const selfChatHit = skipSelfChatHasCheck
  ? false
  : params.selfChatCache?.has({ ...selfChatLookup, text: bodyText });

The accompanying tests explicitly assert this behavior (reflected inbound is dispatched when destination_caller_id is absent).

Recommendation

Prevent reflected duplicates from being dispatched even when destination_caller_id is missing.

Options (pick one consistent with desired product behavior):

Always remember is_from_me=true messages in selfChatCache (as before), but make the cache key strict enough to avoid DM false positives (e.g., include chat_identifier/destination_caller_id/thread identifiers when available).
Add an additional reflection-drop guard: if a message is is_from_me=false but its (sender, chat_id/chat_guid/chat_identifier, created_at, text) matches a recently seen is_from_me=true message (tracked in a dedicated cache), drop it regardless of whether it's considered “self-chat”.

Example approach (conceptual):

// record outbound rows even if not self-chat
if (params.message.is_from_me) {
  params.selfChatCache?.remember(selfChatLookup);
  return isSelfChat ? continueDecision : { kind: "drop", reason: "from me" };
}

// drop reflections for any remembered outbound row
if (params.selfChatCache?.has({ ...selfChatLookup, text: bodyText })) {
  return { kind: "drop", reason: "outbound reflection" };
}

Also consider persistence across restarts (or a longer TTL) if the loop risk is unacceptable when echoCache is cold.

Analyzed PR: #63989 at commit 3bca20a

_{Last updated on: 2026-04-10T08:15:13Z}

…tion (openclaw#63980) Made-with: Cursor

obviyus

Verified the missing-destination_caller_id DM/self-chat misclassification and the reflected-copy loss path.

Maintainer follow-up: scoped selfChatCache.remember() to confirmed self-chat and updated the self-chat regression fixtures after rebasing onto main.

Local gate: pnpm test:extension imessage.

obviyus · 2026-04-10T08:06:17Z

Landed on main.

Landed source commit: 3bca20a
Merge commit: 8ed7c95

Thanks @neeravmakwana.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d6034c23ee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-10T08:08:39Z

  const matchesSelfChatDestination =
-    destinationCallerIdNormalized == null || destinationCallerIdNormalized === senderNormalized;
+    destinationCallerIdNormalized != null && destinationCallerIdNormalized === senderNormalized;
  const isSelfChat =


Preserve self-chat delivery when destination ID is absent

This classification now requires destination_caller_id to be present and equal to sender, so true self-chat rows that omit destination are treated as non-self-chat and then dropped via the is_from_me path. Fresh evidence in this commit is that selfChatCache.remember(...) was moved inside if (isSelfChat), so there is no longer a cache fallback for these rows; delivery now depends on an additional is_from_me=false reflection copy arriving, and environments that emit only the from-me row will lose user self-chat messages.

Useful? React with 👍 / 👎.

@neeravmakwana

…) (thanks @neeravmakwana) * fix(imessage): require destination_caller_id for self-chat classification (#63980) Made-with: Cursor * fix(imessage): scope self-chat cache to self-chat --------- Co-authored-by: Ayaan Zaidi <[email protected]>

@neeravmakwana