fix(bluebubbles): accept updated-message events carrying attachments by coletebou · Pull Request #66105 · openclaw/openclaw

coletebou · 2026-04-13T18:17:16Z

Summary

BlueBubbles fires new-message first (text only), then updated-message ~2-3s later once attachment data is ready. The webhook filter unconditionally discards updated-message events that are not reactions, silently dropping all inbound images, PDFs, and files.

This lets updated-message events through when data.attachments is non-empty, and strips the text body to prevent duplicate delivery (the text was already dispatched via new-message and the 500ms debounce window is too short to coalesce the ~2-3s delayed update).

Supersedes #63983 which had several issues flagged in review:

Boolean-named variable assigned an array
Checked root payload.attachments which could pass the filter but fail normalizeWebhookMessage
No deduplication guard for text+attachment sends

Changes

extensions/bluebubbles/src/monitor.ts:
- Detect attachment-bearing updated-message via data.attachments (not root payload)
- Pass these events through the event type filter
- Strip text body to prevent duplicate delivery (text already processed via new-message)

Test plan

Send an image via iMessage to a BB-connected agent — confirm the image is received and processed
Send text + image in the same iMessage — confirm no duplicate text delivery
Verify updated-message events without attachments are still filtered out
Verify reactions still work normally

BlueBubbles fires new-message first (text only), then updated-message ~2-3s later once attachment data is ready. The webhook filter unconditionally discards updated-message events that are not reactions, silently dropping all inbound images, PDFs, and files. Let updated-message events through when data.attachments is non-empty. Strip the text body from these events to prevent duplicate delivery, since the text was already dispatched via new-message and the 500ms debounce window is too short to coalesce the ~2-3s delayed update. Only checks data.attachments (not root payload.attachments) because normalizeWebhookMessage requires a message container; root-only attachment payloads would pass the filter but fail normalization.

greptile-apps · 2026-04-13T18:20:31Z

Greptile Summary

This PR fixes a silent attachment-drop bug in the BlueBubbles webhook handler: updated-message events carrying data.attachments now pass the event-type filter and are processed as messages with their text body stripped (to prevent duplicate delivery of text already sent via new-message). The fix is well-reasoned and correctly uses payload.data.attachments rather than root payload.attachments to match what normalizeWebhookMessage actually normalizes.

Confidence Score: 5/5

Safe to merge; all findings are P2 style/test suggestions that do not affect correctness.

No P0 or P1 issues found. The logic is correct — isAttachmentUpdate is scoped to updated-message events, the attachment check targets the right nested path, and the text-strip is intentional and documented. Two P2 items remain: a redundant asRecord wrapper and missing test coverage for the new code paths.

extensions/bluebubbles/src/monitor.ts — the two P2 comments above are both in this file.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/bluebubbles/src/monitor.ts
Line: 258

Comment:
**Redundant outer `asRecord` call**

`payload` is already typed as `Record<string, unknown>` (from `const payload = asRecord(parsed.value) ?? {}` at line 225), so wrapping it in `asRecord(...)` again is a no-op. The `as Record<string, unknown> | undefined` cast is also unnecessary since `?.` handles `null` from `asNullableRecord` at runtime. Simplifying this makes the intent clearer:

```suggestion
      const dataAttachments = asRecord(payload.data)?.attachments;
```

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/bluebubbles/src/monitor.ts
Line: 251-285

Comment:
**No tests for the new attachment-update code paths**

The test helpers (`createNewMessagePayloadForTest`, `dispatchWebhookPayloadForTest`) are already in place in `monitor.webhook.test-helpers.ts` and could be extended with an `updated-message` variant to cover: (1) attachment-only updates reaching the message processor, (2) text being stripped to prevent duplicates, and (3) attachment-less `updated-message` events still being filtered out. These three scenarios are exactly the test-plan bullets in the PR description and are currently untested.

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

_{Reviews (1): Last reviewed commit: "fix(bluebubbles): accept updated-message..." | Re-trigger Greptile}

greptile-apps · 2026-04-13T18:20:35Z

+      // -- normalizeWebhookMessage requires a message container
+      // (payload.data/message/payload/event); root-only attachment payloads would
+      // pass the filter but fail normalization.
+      const dataAttachments = (asRecord(asRecord(payload)?.data) as Record<string, unknown> | undefined)?.attachments;


Redundant outer asRecord call

payload is already typed as Record<string, unknown> (from const payload = asRecord(parsed.value) ?? {} at line 225), so wrapping it in asRecord(...) again is a no-op. The as Record<string, unknown> | undefined cast is also unnecessary since ?. handles null from asNullableRecord at runtime. Simplifying this makes the intent clearer:

Suggested change

const dataAttachments = (asRecord(asRecord(payload)?.data) as Record<string, unknown> | undefined)?.attachments;

const dataAttachments = asRecord(payload.data)?.attachments;

Prompt To Fix With AI

This is a comment left during a code review. Path: extensions/bluebubbles/src/monitor.ts Line: 258 Comment: **Redundant outer `asRecord` call** `payload` is already typed as `Record<string, unknown>` (from `const payload = asRecord(parsed.value) ?? {}` at line 225), so wrapping it in `asRecord(...)` again is a no-op. The `as Record<string, unknown> | undefined` cast is also unnecessary since `?.` handles `null` from `asNullableRecord` at runtime. Simplifying this makes the intent clearer: ```suggestion const dataAttachments = asRecord(payload.data)?.attachments; ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-04-13T18:20:36Z

@@ -265,7 +276,13 @@ export async function handleBlueBubblesWebhookRequest(
        }
        return true;
      }
-      const message = reaction ? null : normalizeWebhookMessage(payload);
+      let message = reaction ? null : normalizeWebhookMessage(payload);
+      // Strip text from attachment updates to prevent duplicate delivery.
+      // The text was already dispatched via new-message; the debounce window (500ms)
+      // is shorter than the updated-message delay (~2-3s) so they won't coalesce.
+      if (message && isAttachmentUpdate) {
+        message = { ...message, text: "" };
+      }


No tests for the new attachment-update code paths

The test helpers (createNewMessagePayloadForTest, dispatchWebhookPayloadForTest) are already in place in monitor.webhook.test-helpers.ts and could be extended with an updated-message variant to cover: (1) attachment-only updates reaching the message processor, (2) text being stripped to prevent duplicates, and (3) attachment-less updated-message events still being filtered out. These three scenarios are exactly the test-plan bullets in the PR description and are currently untested.

Prompt To Fix With AI

This is a comment left during a code review. Path: extensions/bluebubbles/src/monitor.ts Line: 251-285 Comment: **No tests for the new attachment-update code paths** The test helpers (`createNewMessagePayloadForTest`, `dispatchWebhookPayloadForTest`) are already in place in `monitor.webhook.test-helpers.ts` and could be extended with an `updated-message` variant to cover: (1) attachment-only updates reaching the message processor, (2) text being stripped to prevent duplicates, and (3) attachment-less `updated-message` events still being filtered out. These three scenarios are exactly the test-plan bullets in the PR description and are currently untested. How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a544b94cc

ℹ️ 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-13T18:21:47Z

+      if (message && isAttachmentUpdate) {
+        message = { ...message, text: "" };
+      }


Keep mention-bearing text on attachment updates

Clearing message.text for every attachment-bearing updated-message causes group mention gating to drop those updates when requireMention is enabled. processMessage evaluates mentions from message.text (not the placeholder), so after this rewrite the attachment follow-up is treated as unmentioned and returns early, which still silently drops media in mention-gated groups even when the original text message mentioned the agent.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-13T18:21:47Z

+      const dataAttachments = (asRecord(asRecord(payload)?.data) as Record<string, unknown> | undefined)?.attachments;
+      const isAttachmentUpdate =
+        eventType === "updated-message" && Array.isArray(dataAttachments) && dataAttachments.length > 0;


Detect attachment updates across all normalized payload shapes

The attachment gate only inspects payload.data.attachments, but normalizeWebhookMessage accepts message containers in multiple forms (payload.message, payload.data.message, and stringified variants via extractMessagePayload). For valid updated-message payloads where attachments live under data.message.attachments (or parsed from string data), isAttachmentUpdate stays false and this event is still filtered out as a non-reaction.

Useful? React with 👍 / 👎.

coletebou · 2026-04-13T18:54:05Z

Closing — review caught that stripping text breaks mention gating in group chats. Resubmitting with a fix that preserves text for mention evaluation.

openclaw-barnacle Bot added channel: bluebubbles Channel integration: bluebubbles size: XS labels Apr 13, 2026

greptile-apps Bot reviewed Apr 13, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 13, 2026

View reviewed changes

coletebou closed this Apr 13, 2026

coletebou mentioned this pull request Apr 13, 2026

fix(bluebubbles): accept updated-message events carrying attachments #66120

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(bluebubbles): accept updated-message events carrying attachments#66105

fix(bluebubbles): accept updated-message events carrying attachments#66105
coletebou wants to merge 1 commit intoopenclaw:mainfrom
coletebou:fix/bb-attachment-update-v2

coletebou commented Apr 13, 2026

Uh oh!

greptile-apps Bot commented Apr 13, 2026

Uh oh!

greptile-apps Bot Apr 13, 2026

Uh oh!

greptile-apps Bot Apr 13, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 13, 2026

Uh oh!

chatgpt-codex-connector Bot Apr 13, 2026

Uh oh!

coletebou commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	const dataAttachments = (asRecord(asRecord(payload)?.data) as Record<string, unknown> \| undefined)?.attachments;
	const dataAttachments = asRecord(payload.data)?.attachments;

Uh oh!

Conversation

coletebou commented Apr 13, 2026

Summary

Changes

Test plan

Uh oh!

greptile-apps Bot commented Apr 13, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

greptile-apps Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

coletebou commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant