fix(imessage): coalesce split-sends without delaying normal DMs by omarshahine · Pull Request #90795 · openclaw/openclaw

omarshahine · 2026-06-06T00:46:02Z

Problem

coalesceSameSenderDms debounced every DM for the full window (2500ms) before replying, because the monitor couldn't know mid-stream whether a second bubble was coming. Two symptoms:

Latency on every reply — even a single normal message ("ok thanks") waited out the full debounce window before the agent began.
Split-sends could still miss — when Apple delivers Dump <URL> as two chat.db rows and the payload row arrives late (e.g. attachment-transfer lag pushing it past the window), the lead-in flushed alone → two turns → a stray "send me the URL" bubble.

Fix

Classify each DM instead of holding all of them:

lead-in — a short bare command-style fragment (Dump, Save this) that plausibly precedes a payload. Briefly held.
payload-join — a URL/attachment that completes a pending lead-in. Merged into the held lead-in and flushed immediately.
instant — everything else (prose, questions, lone URLs). Zero added latency.

Net effect: normal conversation replies instantly; real split-sends still coalesce into one agent turn and now flush as soon as the payload row lands rather than waiting out the window. The debounce window now only bounds how long a lone lead-in waits for a follow-up.

Group chats and the coalesceSameSenderDms: false path are unchanged.

Implementation

New pure helpers in coalesce.ts: iMessageTextHasUrl and isIMessageSplitLeadIn (heuristic: short, no URL, no media, no terminal punctuation), with unit tests.
monitor-provider.ts classifies each DM (classifyDmCoalesce), tracks pending lead-in keys, tags the debouncer entry, and flushes the merged turn immediately on payload-join. The shared inbound-debounce flush-then-separate semantics are left untouched (intentional for control commands).

Tests

coalesce.test.ts: +7 cases covering URL detection and lead-in classification (bare commands, terminal punctuation, word-cap, payload-carrying, media, empty).
Full iMessage monitor suite green (225 tests); extension typecheck + oxlint clean.

🤖 Generated with Claude Code

clawsweeper · 2026-06-06T00:47:40Z

Codex review: needs real behavior proof before merge. Reviewed June 5, 2026, 8:53 PM ET / 00:53 UTC.

Summary
This PR narrows iMessage DM coalescing to hold only short lead-ins, immediately flush joined URL/media payloads, adds coalescing helper tests, and adds an unreleased changelog entry.

PR surface: Source +132, Tests +51, Docs +6. Total +189 across 4 files.

Reproducibility: yes. from source for current behavior: current main debounces every opted-in iMessage DM, matching the PR's latency complaint. I did not establish a live Messages reproduction of the delayed or late-payload split-send path.

Review metrics: 1 noteworthy metric.

Existing config behavior: 1 changed, 0 added, 0 removed. The PR changes the semantics of channels.imessage.coalesceSameSenderDms, so maintainers need upgrade and documentation proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted real iMessage proof for a standalone DM and a Dump plus URL/media split-send after the patch.
[P1] Fix the iMessage monitor debouncer mocks/tests so the new payload-join flushKey path is covered.
Update iMessage docs for the narrowed config semantics and remove the direct CHANGELOG.md edit.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body reports unit/monitor suite and lint/typecheck results, but it does not include redacted real Messages/imsg output showing standalone DM latency and split-send coalescing after the patch; screenshots, terminal output, logs, or a recording should redact private handles, phone numbers, endpoints, and tokens. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A real desktop/transport proof would materially help because the PR's value is visible iMessage latency and split-send behavior that mocks do not prove. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify iMessage standalone DM dispatches without 2.5s delay and Dump + URL split-send merges into one agent turn.

Risk before merge

[P1] Existing users with coalesceSameSenderDms: true may lose the broad rapid same-sender DM batching semantics and now get separate agent turns unless the messages match the new lead-in plus URL/media heuristic.
[P1] No redacted real Messages/imsg proof is provided yet, so the claimed latency improvement and split-send merge behavior are not shown in a real setup.
[P1] The PR carries the protected maintainer label, so it should stay open for explicit maintainer handling even after mechanical fixes.

Maintainer options:

Refresh contract before merge (recommended)
Update docs and tests to state and prove the narrowed coalesceSameSenderDms behavior, remove the changelog edit, and add redacted real iMessage proof before landing.
Accept the narrower batching contract
Maintainers can intentionally accept that existing users lose broad rapid-DM batching, but the PR should make that upgrade-visible in docs and release-note context outside CHANGELOG.md.
Pause if broad batching must remain
If the existing opt-in contract must continue batching arbitrary rapid DMs, pause this PR and redesign around an explicit compatibility mode or a separate stricter option.

Next step before merge

[P1] Needs contributor real iMessage proof plus maintainer acceptance of the changed coalesceSameSenderDms contract; the mechanical fixes are clear but automation cannot supply the contributor's real setup proof.

Security
Cleared: The diff touches iMessage TypeScript behavior, tests, and changelog text only; I found no concrete security or supply-chain regression.

Review findings

[P2] Update the iMessage debouncer mocks for flushKey — extensions/imessage/src/monitor/monitor-provider.ts:1137
[P2] Update the coalescing docs with the narrowed behavior — extensions/imessage/src/monitor/monitor-provider.ts:459-460
[P3] Remove the release-owned changelog edit — CHANGELOG.md:5-9

Review details

Best possible solution:

Keep the selective coalescing direction only if maintainers accept the narrowed config contract, then update docs, fix the iMessage monitor mocks/tests, remove the changelog edit, and add redacted real iMessage proof before merge.

Do we have a high-confidence way to reproduce the issue?

Yes from source for current behavior: current main debounces every opted-in iMessage DM, matching the PR's latency complaint. I did not establish a live Messages reproduction of the delayed or late-payload split-send path.

Is this the best way to solve the issue?

Unclear until maintainers accept the narrowed config contract. The implementation layer is plausible, but the PR needs docs, tests/mocks, changelog cleanup, and real iMessage proof before it can be considered the best fix.

Full review comments:

[P2] Update the iMessage debouncer mocks for flushKey — extensions/imessage/src/monitor/monitor-provider.ts:1137
This new call runs for payload-join messages. The existing monitor.last-route.test.ts debouncer mock only exposes enqueue, and that file already sends Dump followed by a URL with coalesceSameSenderDms: true, so the test path should throw before it proves the behavior. Add flushKey to the iMessage monitor mocks or use the real helper in that test.
Confidence: 0.9
[P2] Update the coalescing docs with the narrowed behavior — extensions/imessage/src/monitor/monitor-provider.ts:459-460
This branch changes coalesceSameSenderDms from holding every DM to holding only lead-ins and joined payloads, but docs/channels/imessage.md still tells users that every DM waits and that consecutive same-sender rows are merged. Update the user docs with the new latency and upgrade semantics before merge.
Confidence: 0.88
[P3] Remove the release-owned changelog edit — CHANGELOG.md:5-9
Root policy keeps CHANGELOG.md release-owned for generated release notes. This normal PR should keep the release-note context in the PR body or squash/merge message instead of adding an Unreleased section that release generation will own.
Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This is a normal-priority iMessage behavior fix with limited channel scope, but it has blocking test/docs/proof issues before merge.
add merge-risk: 🚨 compatibility: Merging changes the behavior of an existing opt-in iMessage config from broad same-sender DM batching to a narrower lead-in/payload heuristic.
add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports unit/monitor suite and lint/typecheck results, but it does not include redacted real Messages/imsg output showing standalone DM latency and split-send coalescing after the patch; screenshots, terminal output, logs, or a recording should redact private handles, phone numbers, endpoints, and tokens. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

P2: This is a normal-priority iMessage behavior fix with limited channel scope, but it has blocking test/docs/proof issues before merge.
merge-risk: 🚨 compatibility: Merging changes the behavior of an existing opt-in iMessage config from broad same-sender DM batching to a narrower lead-in/payload heuristic.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports unit/monitor suite and lint/typecheck results, but it does not include redacted real Messages/imsg output showing standalone DM latency and split-send coalescing after the patch; screenshots, terminal output, logs, or a recording should redact private handles, phone numbers, endpoints, and tokens. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +132, Tests +51, Docs +6. Total +189 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	158	26	+132
Tests	1	51	0	+51
Docs	1	6	0	+6
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	215	26	+189

What I checked:

Root repository policy applied: Root AGENTS.md was read fully; its docs-with-behavior and release-owned changelog guidance affect this review. (AGENTS.md:1, e5d1fadea7e2)
Scoped extension policy applied: extensions/AGENTS.md was read fully; the PR stays inside the iMessage plugin boundary and does not add a core seam. (extensions/AGENTS.md:1, e5d1fadea7e2)
Current main debounces every opted-in DM: Current main returns msg.is_group !== true when coalesceSameSenderDms is enabled, so normal DMs and split-send candidates all wait for the configured debounce window. (extensions/imessage/src/monitor/monitor-provider.ts:406, e5d1fadea7e2)
PR introduces explicit flushKey call: The PR head calls await inboundDebouncer.flushKey(dm.key) for payload joins after enqueueing the payload. (extensions/imessage/src/monitor/monitor-provider.ts:1137, feaa685710bf)
Existing iMessage monitor mock lacks flushKey: The current monitor.last-route.test.ts mock debouncer exposes only enqueue, but its coalesced live bucket test sends Dump then a URL with coalesceSameSenderDms: true, which exercises the new payload-join path. (extensions/imessage/src/monitor.last-route.test.ts:78, e5d1fadea7e2)
Shared debouncer supports flushKey in production: The production debouncer does expose flushKey, so the blocker is the PR's unupdated test/mocking surface rather than a missing runtime API. (src/auto-reply/inbound-debounce.ts:170, e5d1fadea7e2)

Likely related people:

Ayaan Zaidi: Current-main blame/log attribute the iMessage monitor, coalesce helper, and affected test mocks to commit 21512a6. (role: introduced behavior and recent area contributor; confidence: high; commits: 21512a696ff1; files: extensions/imessage/src/monitor/monitor-provider.ts, extensions/imessage/src/monitor/coalesce.ts, extensions/imessage/src/monitor.last-route.test.ts)
Omar Shahine: Beyond this PR, local history shows prior merged BlueBubbles channel attachment/catchup fixes, which are adjacent to this iMessage split-send and media-routing behavior. (role: adjacent channel contributor and PR author; confidence: medium; commits: 77d9fd693f18, 6f1d321aabab; files: extensions/bluebubbles/src/monitor.ts, extensions/bluebubbles/src/attachments.ts, extensions/bluebubbles/src/catchup.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

`coalesceSameSenderDms` previously debounced every DM for the full window (2500ms), so each reply waited out the debounce even when no follow-up was coming. A `Dump <URL>` whose payload row arrived late (e.g. attachment transfer lag) could also miss the window and dispatch as two turns, producing a stray "send me the URL" bubble. Classify each DM instead of holding all of them: - lead-in: a short bare command-style fragment (`Dump`) that plausibly precedes a payload — briefly held. - payload-join: a URL/attachment that completes a pending lead-in — merged into the held lead-in and flushed immediately. - instant: everything else (prose, questions, lone URLs) — zero added latency. Normal conversation now replies instantly; real split-sends still coalesce and flush as soon as the payload row lands rather than waiting out the window. The window now only bounds how long a lone lead-in waits for a follow-up. Adds pure `isIMessageSplitLeadIn` / `iMessageTextHasUrl` helpers in coalesce.ts with unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

omarshahine · 2026-06-06T04:54:23Z

@clawsweeper re-review

clawsweeper · 2026-06-06T04:54:26Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/27053127891
Updated: 2026-06-06T05:02:33.799Z

omarshahine · 2026-06-06T05:00:28Z

Closing this for now. The selective OpenClaw coalescing heuristic improves latency, but it still guesses from short lead-in text because the imsg JSON/RPC payload does not expose the structural row metadata OpenClaw needs.

Better path: fix imsg first by exposing an additive structural signal for URL/link-preview or split-send payload rows, validate that against a real Messages setup, then return to OpenClaw with a smaller consumer change that does not rely on command-like short-message inference.

I’ll reopen or replace this after the imsg contract is fixed and tested end to end.

…th back-compat (#90858) Gate iMessage same-sender DM split-send coalescing on imsg's structural `balloon_bundle_id` URL-balloon marker (openclaw/imsg#137) instead of timing/ text-shape inference, with a session capability latch and a back-compat path: - URL-balloon marker present -> merge (precise split-send). - Build known to emit balloon metadata (session latch) -> keep non-marker buckets separate (the precision win). - Build that never emits balloon metadata (older imsg) -> preserve the legacy unconditional merge, so split-send users do not regress to two turns. Never merges more than shipped main already did. Verified live end-to-end: the patched gateway, watching a real chat.db via an imsg #137 build, merged a real iPhone-sent `Dump <url>` split-send into one turn. Client-side removal once imsg coalesces upstream is tracked in #91243 (openclaw/imsg#141). Closes #90795

…th back-compat (openclaw#90858) Gate iMessage same-sender DM split-send coalescing on imsg's structural `balloon_bundle_id` URL-balloon marker (openclaw/imsg#137) instead of timing/ text-shape inference, with a session capability latch and a back-compat path: - URL-balloon marker present -> merge (precise split-send). - Build known to emit balloon metadata (session latch) -> keep non-marker buckets separate (the precision win). - Build that never emits balloon metadata (older imsg) -> preserve the legacy unconditional merge, so split-send users do not regress to two turns. Never merges more than shipped main already did. Verified live end-to-end: the patched gateway, watching a real chat.db via an imsg openclaw#137 build, merged a real iPhone-sent `Dump <url>` split-send into one turn. Client-side removal once imsg coalesces upstream is tracked in openclaw#91243 (openclaw/imsg#141). Closes openclaw#90795

openclaw-barnacle Bot added channel: imessage Channel integration: imessage size: M maintainer Maintainer-authored PR labels Jun 6, 2026

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 6, 2026

omarshahine force-pushed the fix/imessage-selective-dm-coalesce branch from feaa685 to fc5899d Compare June 6, 2026 04:32

docs(imessage): align coalesceSameSenderDms contract with selective hold

e7f8c87

openclaw-barnacle Bot added the docs Improvements or additions to documentation label Jun 6, 2026

omarshahine added 2 commits June 5, 2026 21:44

test(imessage): add flushKey to debouncer mocks for payload-join path

063fdc3

docs(imessage): document classify ordering invariant for split-send race

8fe7a53

omarshahine self-assigned this Jun 6, 2026

omarshahine closed this Jun 6, 2026

omarshahine mentioned this pull request Jun 6, 2026

fix(imessage): gate split-send coalescing on imsg metadata #90858

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(imessage): coalesce split-sends without delaying normal DMs#90795

fix(imessage): coalesce split-sends without delaying normal DMs#90795
omarshahine wants to merge 4 commits into
mainfrom
fix/imessage-selective-dm-coalesce

omarshahine commented Jun 6, 2026

Uh oh!

clawsweeper Bot commented Jun 6, 2026 •

edited

Loading

Uh oh!

omarshahine commented Jun 6, 2026

Uh oh!

clawsweeper Bot commented Jun 6, 2026 •

edited

Loading

Uh oh!

omarshahine commented Jun 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

omarshahine commented Jun 6, 2026

Problem

Fix

Implementation

Tests

Uh oh!

clawsweeper Bot commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omarshahine commented Jun 6, 2026

Uh oh!

clawsweeper Bot commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omarshahine commented Jun 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jun 6, 2026 •

edited

Loading

clawsweeper Bot commented Jun 6, 2026 •

edited

Loading