Greptile Summary

This PR fixes three genuine iMessage echo-loop bugs (wrong isSelfChat classification, short 4 s TTL, and U+FFFD mismatch in the echo cache). The TTL increase and the normalizeEchoTextKey regex change are correct. However, two regressions block the primary fix from working.

• destination_caller_id never declared in IMessagePayload (extensions/imessage/src/monitor/types.ts): inbound-processing.ts line 218 reads params.message.destination_caller_id, but the type has no such field. TypeScript will fail to compile, and at runtime the value is always undefined, so !destCallerId is always true — collapsing the new self-chat guard back to the old broken senderMatchesChatId-only check.
• deliver.ts scope double-adds "imessage:" prefix: target for a DM is already "imessage:<sender>" (from buildIMessageInboundContext), so the new \${accountId}:imessage:${target}`produces"accountId:imessage:imessage:", which never matches buildIMessageEchoScope's "accountId:imessage:". For groups the mismatch is identical. The existing deliver.test.ts` assertion (line 148) catches this. The original one-liner was correct.

Confidence Score: 3/5

Not safe to merge — two P1 regressions leave the echo-loop fix non-functional and break the TypeScript build.

The missing destination_caller_id field causes a compile failure and a silent runtime no-op that neutralises the isSelfChat fix. The scope double-prefix in deliver.ts defeats all echo-cache deduplication after merge. Both defects are on the directly changed path and must be fixed before the PR can ship.

extensions/imessage/src/monitor/types.ts (missing destination_caller_id declaration) and extensions/imessage/src/monitor/deliver.ts (incorrect scope prefix)

This is a comment left during a code review.
Path: extensions/imessage/src/monitor/types.ts
Line: 10-27

Comment:
**`destination_caller_id` missing from `IMessagePayload`**

`inbound-processing.ts` reads `params.message.destination_caller_id` (line 218 of the changed file), but `IMessagePayload` has no such field. TypeScript will reject this at compile time, and even if the build somehow passed, the runtime value would always be `undefined` — making `!destCallerId` unconditionally `true` and collapsing the new guard back to the old broken `senderMatchesChatId`-only check. The field must be declared here for Bug 1's fix to have any effect.

```suggestion
export type IMessagePayload = {
  id?: number | null;
  guid?: string | null;
  chat_id?: number | null;
  sender?: string | null;
  is_from_me?: boolean | null;
  text?: string | null;
  reply_to_id?: number | string | null;
  reply_to_text?: string | null;
  reply_to_sender?: string | null;
  created_at?: string | null;
  attachments?: IMessageAttachment[] | null;
  chat_identifier?: string | null;
  chat_guid?: string | null;
  chat_name?: string | null;
  participants?: string[] | null;
  is_group?: boolean | null;
  destination_caller_id?: string | null;
};
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/imessage/src/monitor/deliver.ts
Line: 31-34

Comment:
**Echo-cache scope double-adds `"imessage:"` prefix**

For a DM, `target` is already `"imessage:<sender>"` (set by `buildIMessageInboundContext` as `` imessageTo = `imessage:${decision.sender}` ``). Prepending another `"imessage:"` produces scope `"accountId:imessage:imessage:<sender>"`, which never matches the key `buildIMessageEchoScope` returns (`"accountId:imessage:<sender>"`). For groups `target` is `"chat_id:<n>"` and the new scope `"accountId:imessage:chat_id:<n>"` likewise never matches `"accountId:chat_id:<n>"`. The existing test (`deliver.test.ts` line 148) already asserts the correct format — `"acct-3:chat_id:30"` for `target: "chat_id:30"` — and would catch this. The original `scope = \`${accountId ?? ""}:${target}\`` was already correct; the added prefix silently breaks echo deduplication for every conversation.

```suggestion
  const scope = `${accountId ?? ""}:${target}`;
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(imessage): prevent echo loop from se..." | Re-trigger Greptile}

Re: P2 on TTL increase — acknowledged tradeoff. The 30s window does widen the false-positive path for text-only matching on short repeated messages like "ok".

However, the 4s TTL causes echo loops that flood users with 2-10+ duplicate replies — a much worse failure mode. The text-only matching path only fires when there's no usable message ID (normalized to null/ok/unknown), which is uncommon in normal flow.

A follow-up could add a minimum text length threshold to skip text-only matching for very short strings, but keeping that separate from this fix.

Rebased — review comments no longer applicable

This PR has been rebased to only include the remaining unmerged fix: U+FFFD garbage char normalization in normalizeEchoTextKey() (echo-cache.ts).

The previous fixes for TTL (4s → 30s) and destination_caller_id self-chat detection have been removed from this PR — they were independently merged upstream in #61619, #63868, #63980, #63989, #64000 and ship in 2026.4.10.

The three P1 review comments from Greptile and ChatGPT Codex all reference files (deliver.ts, inbound-processing.ts) that are no longer in the diff:

• deliver.ts scope double-prefix — file no longer changed
• inbound-processing.ts null guard on chatIdentifier — file no longer changed

The only change remaining is a one-line regex addition in echo-cache.ts to strip U+FFFD/C0/C1 control characters before echo text-key comparison. See #61312, #61821.

ProjectClownfish pushed a narrow repair to this branch so the original contributor path can stay canonical.

Source PR: #62191
Validation: pnpm test extensions/imessage/src/monitor/monitor-provider.echo-cache.test.ts; pnpm check:changed
Contributor credit is preserved in the branch history and PR context.

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve.

Current main and the latest release still lack the requested iMessage echo-cache normalization for leading U+FFFD/control-byte corruption, so this PR is not obsolete. The PR remains a relevant implementation candidate, but the provided current diff appears to need a regex syntax repair before it can land safely.

Best possible solution:

Keep this PR open for maintainer follow-up. The best path is to repair the leading-only regex using syntax-safe U+FFFD/U+FFFE/U+FFFF and C0/C1 ranges, keep the interior-control preservation tests, coordinate the overlap with #63581, and then land this or an equivalent iMessage plugin fix in extensions/imessage/src/monitor/echo-cache.ts.

What I checked:

• current-main-normalization-gap: Current main's normalizeEchoTextKey() only converts CRLF to LF and trims whitespace; it does not strip leading U+FFFD/C0/C1 corruption bytes before sent-message echo-cache comparison. (extensions/imessage/src/monitor/echo-cache.ts:26, 955b4df0939f)
• current-tests-gap: The focused echo-cache test file covers CRLF/trim matching, message-id matching, placeholder IDs, and TTL behavior, but it has no leading U+FFFD/control-prefix regression test or interior-control preservation test on current main. (extensions/imessage/src/monitor/monitor-provider.echo-cache.test.ts:9, 955b4df0939f)
• latest-release-same-gap: The latest release tag v2026.4.26 points at be8c246, and that release copy of echo-cache.ts has the same CRLF-plus-trim normalizeEchoTextKey() implementation without leading corruption-byte stripping. (extensions/imessage/src/monitor/echo-cache.ts:26, be8c24633aaa)
• pr-diff-functional-blocker: The provided current PR diff's regex literal /^[\ufffd\ufffe\uffff^@-^\u007f-^]+/ is rejected by Node with SyntaxError: Range out of order in character class, so the branch likely needs a syntax-safe expression before landing. (extensions/imessage/src/monitor/echo-cache.ts:28, eec4bb724d08)
• earlier-review-blockers-addressed-on-main: The earlier broad PR blockers are not reasons to close the narrowed PR: current main declares destination_caller_id, requires an explicit matching destination for self-chat classification, and keeps sent-message cache scopes as ${accountId ?? ""}:${target} in deliverReplies(). (extensions/imessage/src/monitor/types.ts:15, 955b4df0939f)
• related-pr-overlap: Related open PR imessage: strip NUL bytes from echo-cache text normalization #63581 also targets echo-cache prefix normalization for NUL bytes. That overlap supports coordination, not closing this PR, because current main still lacks the U+FFFD/control-prefix behavior requested here.

Likely related people:

• vincentkoc: Current main's changelog credits @vincentkoc on the earlier iMessage self-chat echo dedupe work in fix(imessage): dedupe reflected self-chat duplicates #38440, and the PR discussion says vincentkoc pushed the latest narrow repair to this branch. (role: feature-history owner and recent PR maintainer; confidence: high; commits: eec4bb724d08, #38440; files: extensions/imessage/src/monitor/echo-cache.ts, extensions/imessage/src/monitor/self-chat-dedupe.test.ts, CHANGELOG.md)
• neeravmakwana: Merged iMessage self-chat and destination_caller_id fixes fix(imessage): avoid DM self-chat false positives #61619 and fix(imessage): require destination_caller_id for self-chat classification (#63980) #63989 are directly adjacent to the echo-cache path and are credited in the current changelog; current main source still carries that destination_caller_id behavior. (role: adjacent merged-feature contributor; confidence: medium; commits: #61619, #63989; files: extensions/imessage/src/monitor/inbound-processing.ts, extensions/imessage/src/monitor/types.ts, extensions/imessage/src/monitor/self-chat-dedupe.test.ts)
• Peter Steinberger [email protected]: Local blame/log on the available current-main history attributes the current echo-cache.ts snapshot and latest release changelog consolidation to Peter Steinberger, making him a plausible maintainer routing fallback for this area. (role: recent maintainer on current main snapshot; confidence: low; commits: 3aac8e650c50, be8c24633aaa; files: extensions/imessage/src/monitor/echo-cache.ts, extensions/imessage/src/monitor/monitor-provider.echo-cache.test.ts, CHANGELOG.md)

Remaining risk / open question:

• The provided current PR diff appears to contain an invalid JavaScript regex range; this should be repaired before maintainer landing or CI will likely fail at parse time.
• The PR overlaps imessage: strip NUL bytes from echo-cache text normalization #63581 around echo-cache prefix cleanup, so maintainers should combine or coordinate the U+FFFD/control-prefix and NUL-prefix handling to avoid competing normalizers.
• No tests were run because this was a required read-only review; the decision is based on source, tag, discussion, PR-diff, and read-only Node syntax inspection.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 955b4df0939f.

ProjectClownfish could not safely update this branch, so it opened a narrow replacement PR instead.

Replacement PR: #73891
Source PR: #62191
Contributor credit is preserved in the replacement PR body and changelog plan.