Skip to content

[codex] fix(bluebubbles): dedupe webhook replays without dropping edits#52277

Closed
aidenwildenbadt wants to merge 18 commits intoopenclaw:mainfrom
aidenwildenbadt:codex/refresh-bluebubbles-webhook-replay-dedupe
Closed

[codex] fix(bluebubbles): dedupe webhook replays without dropping edits#52277
aidenwildenbadt wants to merge 18 commits intoopenclaw:mainfrom
aidenwildenbadt:codex/refresh-bluebubbles-webhook-replay-dedupe

Conversation

@aidenwildenbadt
Copy link
Copy Markdown

@aidenwildenbadt aidenwildenbadt commented Mar 22, 2026

Recreated from auto-closed #52239 after the active-PR queue dropped back under the repo limit.

Summary

This refreshes the old BlueBubbles replay-dedupe PR as a clean branch off current origin/main.

The replacement carries forward the replay/edit handling work from the original branch:

  • replay dedupe keys now account for real edit metadata and reply context
  • updated-message events bypass debounce merging so rapid edits are not collapsed into one body
  • malformed updated-message webhook noise still returns 200 ok instead of creating retry churn

It also folds in the still-live review feedback from the old PR:

  • media-only updated-message payloads are treated as conversational instead of being dropped as noise
  • explicit edit metadata now wins over UUID-churn filtering, so a legitimate UUID-like edit is preserved
  • replay keys now include the webhook event type, which prevents same-text edit reversions from colliding with the original new-message replay key
  • the modern BlueBubbles facade export surface is kept intact while adding the replay dedupe helper export the branch needed

Validation

  • npx vitest run extensions/bluebubbles/src/monitor.test.ts -t "media-only updated-message|UUID edits when explicit edit metadata is present|text reversions back to the original body|canonical UUID churn text|text edits without explicit edit metadata when text changes"
  • npx vitest run extensions/bluebubbles/src/monitor.test.ts -t "attachment identity changes|updated-message events that differ only by edit metadata|updated-message events when reply metadata appears later"
  • commit-time pnpm check / lint hooks passed during git commit

Contribution Notes

  • AI-assisted: yes (Codex)
  • Testing degree: lightly tested
  • Review thread handling: addressed and resolved where applicable before re-requesting review

@openclaw-barnacle openclaw-barnacle Bot added channel: bluebubbles Channel integration: bluebubbles size: XL labels Mar 22, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 04ef85f768

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 22, 2026

Greptile Summary

This PR fixes BlueBubbles webhook replay deduplication so that rapid replay retries no longer drop legitimate message edits. It introduces a comprehensive buildInboundReplayKey that encodes eventType, edit metadata (itemType/dateEdited), reply context, and content fingerprints, preventing same-text edit reversions from colliding with the original new-message key. updated-message events bypass the debounce window entirely to avoid merging edited bodies, and malformed updated-message payloads return 200 OK instead of driving retry churn. A new replayLifecycle callback pair on BlueBubblesDebounceEntry lets the debounce flush signal success/failure back to the replay tracker, enabling deferred-replay re-queuing on failure. The src/plugin-sdk/bluebubbles.ts facade is extended to export createDedupeCache and beginWebhookRequestPipelineOrReject, keeping the extension's import surface clean. The large test refactor removes the indirect helper facade and instead drives registerBlueBubblesWebhookTarget / handleBlueBubblesWebhookRequest directly, with broad new coverage for UUID-churn filtering, media-only edit handling, and reply-context keying.

Key changes:

  • buildInboundReplayKey now includes eventType, edit metadata, and reply context — eliminating false dedupe collisions across edit generations and between new-message/updated-message events
  • shouldIgnoreUpdatedNonConversationalEvent gates UUID-churn suppression on the absence of explicit edit metadata, preserving legitimate UUID-shaped edits
  • onFlush in the debounce registry wraps both the single and multi-message paths with settleReplayLifecycle to propagate success/failure to replay bookkeeping
  • One minor redundancy: resetBlueBubblesSelfChatCache() is called twice in the test beforeEach (lines 253–254) — harmless but a leftover copy-paste artifact worth removing

Confidence Score: 5/5

  • This PR is safe to merge; the production logic is sound and the single flagged issue is a harmless test redundancy.
  • The replay-dedupe logic is well-designed, the lifecycle callback wiring is correct in all success/failure paths, and the comprehensive test suite validates the key edge cases. The only concrete issue found is a duplicate resetBlueBubblesSelfChatCache() call in beforeEach which is idempotent and has no production impact.
  • No files require special attention beyond the duplicate reset call in monitor.test.ts.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/bluebubbles/src/monitor.test.ts
Line: 253-254

Comment:
**Duplicate `resetBlueBubblesSelfChatCache` call**

`resetBlueBubblesSelfChatCache()` is called twice in a row. This is harmless (the function is idempotent), but it looks like a copy-paste artifact from the old `resetBlueBubblesMonitorTestState` helper which called it internally. The second call can be removed.

```suggestion
    resetBlueBubblesSelfChatCache();
```

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

Reviews (1): Last reviewed commit: "fix(bluebubbles): dedupe webhook replays..." | Re-trigger Greptile

Comment thread extensions/bluebubbles/src/monitor.test.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e1d0c9df5

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c95a288ec

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts
Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 39a7b08797

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts
Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42e98ef180

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Comment thread extensions/bluebubbles/src/monitor.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d5d360f456

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fe20150cb

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Comment thread extensions/bluebubbles/src/monitor.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 759748dff8

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts Outdated
Comment thread extensions/bluebubbles/src/monitor.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f8beb8506

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Comment thread extensions/bluebubbles/src/monitor-debounce.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ac61c30f8

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor-debounce.ts
Comment thread extensions/bluebubbles/src/monitor.ts
@aidenwildenbadt aidenwildenbadt force-pushed the codex/refresh-bluebubbles-webhook-replay-dedupe branch from 4ac61c3 to c9c4042 Compare March 23, 2026 21:14
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d05bb06757

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts Outdated
Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1b9553ae3

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts Outdated
Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b2b5b6e3e

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor-debounce.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bba6a6f884

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7153b99454

ℹ️ 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".

Comment thread extensions/bluebubbles/src/monitor.ts
@aidenwildenbadt aidenwildenbadt force-pushed the codex/refresh-bluebubbles-webhook-replay-dedupe branch from 7153b99 to 10cc9da Compare March 27, 2026 00:28
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10cc9da5f9

ℹ️ 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".

Comment on lines 423 to +427
res,
targetsByPath: webhookTargets,
allowMethods: ["POST"],
rateLimiter: webhookRateLimiter,
rateLimitKey,
inFlightLimiter: webhookInFlightLimiter,
inFlightKey: `${normalizedPath}:${clientIp}`,
handle: async ({ path, targets }) => {
const url = requestUrl;
const guidParam = url.searchParams.get("guid") ?? url.searchParams.get("password");
const headerToken =
req.headers["x-guid"] ??
req.headers["x-password"] ??
req.headers["x-bluebubbles-guid"] ??
req.headers["authorization"];
const guid = (Array.isArray(headerToken) ? headerToken[0] : headerToken) ?? guidParam ?? "";
const target = resolveWebhookTargetWithAuthOrRejectSync({
targets,
res,
isMatch: (target) => {
const token = target.account.config.password?.trim() ?? "";
return safeEqualSecret(guid, token);
},
});
if (!target) {
console.warn(
`[bluebubbles] webhook rejected: status=${res.statusCode} path=${path} guid=${maskSecret(url.searchParams.get("guid") ?? url.searchParams.get("password") ?? "")}`,
);
return true;
}
const body = await readWebhookBodyOrReject({
req,
res,
profile: "post-auth",
invalidBodyMessage: "invalid payload",
});
if (!body.ok) {
console.warn(`[bluebubbles] webhook rejected: status=${res.statusCode}`);
return true;
}
inFlightKey: `${path}:${req.socket.remoteAddress ?? "unknown"}`,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reinstate per-IP rate limiting for webhook ingress

beginWebhookRequestPipelineOrReject is now called without rateLimiter/rateLimitKey, so this endpoint only enforces concurrent in-flight caps and no longer applies fixed-window throttling. Under retry storms or abuse, a single source can send unbounded sequential POSTs and force repeated auth/body parsing work, which is a regression from the previous webhook flood protection.

Useful? React with 👍 / 👎.

Comment on lines 705 to 707
password: account.config.password,
accountId: account.accountId,
timeoutMs: 5000,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pass allowPrivateNetwork when probing BlueBubbles server info

This fetchBlueBubblesServerInfo call dropped allowPrivateNetwork: account.config.allowPrivateNetwork === true. For accounts pointing at localhost/private RFC1918 BlueBubbles hosts, the probe now runs with the default SSRF policy and often returns null, so cached private_api/macOS capability state is never populated and action gating falls back to unknown behavior.

Useful? React with 👍 / 👎.

Comment on lines +752 to +756
export {
_resetBlueBubblesShortIdState,
_resetBlueBubblesWebhookReplayState,
resolveBlueBubblesMessageId,
resolveWebhookPathFromConfig,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-export the webhook security reset helper used in tests

The module export list no longer includes clearBlueBubblesWebhookSecurityStateForTest, but test/helpers/extensions/bluebubbles-monitor.ts still imports it (and monitor.webhook-auth.test.ts depends on that helper). This creates a module-load error (does not provide an export named ...) and breaks that test path.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd959b6471

ℹ️ 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".

Comment on lines 90 to 94
export {
WEBHOOK_RATE_LIMIT_DEFAULTS,
createFixedWindowRateLimiter,
beginWebhookRequestPipelineOrReject,
createWebhookInFlightLimiter,
normalizeWebhookPath,
readWebhookBodyOrReject,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore webhook-ingress exports consumed by BlueBubbles runtime API

This export list change drops WEBHOOK_RATE_LIMIT_DEFAULTS, createFixedWindowRateLimiter, and resolveRequestClientIp from openclaw/plugin-sdk/bluebubbles, but extensions/bluebubbles/src/runtime-api.ts still re-exports those symbols and now also needs the newly used pipeline helpers. That mismatch causes missing-export module linkage errors when ./runtime-api.js is loaded, which prevents the BlueBubbles monitor path from initializing correctly.

Useful? React with 👍 / 👎.

console.warn(`[bluebubbles] webhook rejected: status=${res.statusCode}`);
return true;
}
inFlightKey: `${path}:${req.socket.remoteAddress ?? "unknown"}`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Key in-flight limiting by resolved client IP

The new in-flight key uses req.socket.remoteAddress directly, so in reverse-proxy deployments all users behind one proxy share the same concurrency bucket. With the default in-flight cap, concurrent requests from different clients can now throttle each other even when each client is low volume; previously this path resolved client IPs with trusted proxy rules, which preserved per-client isolation.

Useful? React with 👍 / 👎.

@omarshahine
Copy link
Copy Markdown
Contributor

Hey @aidenwildenbadt — just a heads up that I've opened #66230 which addresses a different (but adjacent) BlueBubbles replay problem: stale new-message webhooks that fire when the BB Server restarts its MessagePoller. That's orthogonal to the edit-metadata replay-key work you have here.

The PR body of #66230 includes a note about how the two interact: today only new-message events flow through processMessage, so GUID-only dedupe can't drop edits — but if this PR's work (routing edit bodies through processMessage) lands after #66230, the dedupe key will need to expand to include event type + edit metadata so an edit is treated as a distinct key from its original. Straightforward forward-compatible follow-up.

This PR is large (~4.7k LoC, mostly a test refactor) which is part of why it's been hard to land. Would it be useful to split the actual production fix for edit-text routing into a small focused PR on its own? That would be much easier to review and get in — and with #66230 carrying the restart-replay concern, the edit-path PR can stay tight to just the edit-handling change it needs. Happy to look at that if you'd like.

@omarshahine
Copy link
Copy Markdown
Contributor

Taking a look at this after #67426 and #66816 land. This addresses a real edge case where updated-message (iMessage edits) shares the same messageId as the original new-message, so the persistent GUID dedupe introduced in #66816 would silently drop legitimate edits. Will review for compatibility with the merged dedup + replay work and assess whether it needs a rebase.

omarshahine added a commit that referenced this pull request Apr 16, 2026
…d-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (#64105, #61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (#67437)

Closes #64105, closes #61861, closes #65430.
omarshahine added a commit that referenced this pull request Apr 16, 2026
…d-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (#64105, #61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (#67437)

Closes #64105, closes #61861, closes #65430.
omarshahine added a commit that referenced this pull request Apr 16, 2026
…d-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (#64105, #61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (#67437)

Closes #64105, closes #61861, closes #65430.
omarshahine added a commit that referenced this pull request Apr 16, 2026
…d-message events (#67510)

* fix(bluebubbles): restore inbound image attachments and accept updated-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (#64105, #61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (#67437)

Closes #64105, closes #61861, closes #65430.

* fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests

- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
  allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
  instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
  asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
  success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.

* fix(ci): update raw-fetch allowlist line number after dispatcher strip

* fix(bluebubbles): resolve PR review findings (#67510)

- monitor-processing: move attachment retry into the !rawBody guard so
  image-only new-message events that arrive with empty attachments and
  empty text are recovered via a BB API refetch before being dropped.
  The existing retry block at the end of processMessageAfterDedupe was
  unreachable for this case because the !rawBody early-return fired
  first. (Greptile)
- monitor: derive isAttachmentUpdate from the normalized message shape
  instead of raw payload.data.attachments so updated-message webhooks
  with attachments under wrapper formats (payload.message, JSON-string
  payloads) are correctly routed through for processing instead of
  silently filtered. (Codex)
- types: use bundled-undici fetch when init.dispatcher is present so
  the SSRF guard's DNS-pinning dispatcher is preserved when this
  function is called as fetchImpl from guarded callers (e.g. the
  attachment download path via fetchRemoteMedia). Falls back to
  globalThis.fetch when no dispatcher is present so tests that stub
  globalThis.fetch keep working. (Codex)
- attachments: blueBubblesPolicy returns undefined for the non-private
  case (matching monitor-processing's helper) so sendBlueBubblesAttachment
  stops routing localhost BB through the SSRF guard. (Greptile)
- scripts/check-no-raw-channel-fetch: bump the types.ts allowlist line
  to match the restructured non-SSRF branch.

* fix(bluebubbles): move attachment retry before rawBody guard, fix stale log

Move the attachment retry block (2s BB API refetch for empty attachments)
before the !rawBody early-return guard. Previously, image-only messages
with text='' and attachments=[] would be dropped by the !rawBody check
before the retry could fire, making fix #4 dead code for its primary
use-case. Now the retry runs first and recomputes the placeholder from
resolved attachments so rawBody becomes non-empty when media is found.

Also fix stale log message that still said 'without reaction' after the
filter was expanded to pass through attachment updates.

* fix(bluebubbles): revert undici import, restore dispatcher-strip approach

Revert the @claude bot's undici import in types.ts — it introduced a
direct 'undici' dependency that is not declared in the BB extension's
package.json and would break isolated plugin installs. Restore the
original dispatcher-strip approach which is correct: the SSRF guard
already completed validation upstream before calling this function as
fetchImpl, so stripping the dispatcher does not weaken security.

* fix(bluebubbles): remove dead empty-body recovery block in !rawBody guard

The empty-body attachment-recovery block added in the earlier PR revision
is now redundant because the main retry block was moved above the rawBody
computation in 0d7d1c4. Worse, that leftover block reassigned the
(now-const) placeholder variable, throwing `TypeError: Assignment to
constant variable` at runtime for image-only messages — breaking the very
recovery path it was meant to protect (flagged by Codex on 4bfc2777).

Remove the dead block; the up-front retry already handles the image-only
case by recovering attachments before the rawBody computation, so once we
reach the !rawBody guard with an empty body it is genuinely empty and
should drop as before.

* fix(ci): update raw-fetch allowlist line after dispatcher-strip revert

279dba1 reverted types.ts back to the dispatcher-strip approach,
which put the `fetch(url, ...)` call at line 189 instead of line 198.
Bump the allowlist entry to match so `lint:tmp:no-raw-channel-fetch`
stops failing check-additional.

* test(pdf-tool): update stale opus-4-6 constant to opus-4-7

`628b454eff feat: default Anthropic to Opus 4.7` bumped the bundled
anthropic image default to `claude-opus-4-7` but missed updating the
`ANTHROPIC_PDF_MODEL` constant in pdf-tool.model-config.test.ts. The
tests now fail on any PR that runs the `checks-node-agentic-agents-plugins`
shard because the resolver returns 4-7 while the test asserts 4-6.

Bump the constant to 4-7 to match the bundled default.

---------

Co-authored-by: Lobster <10343873+omarshahine@users.noreply.github.com>
xudaiyanzi pushed a commit to xudaiyanzi/openclaw that referenced this pull request Apr 17, 2026
…d-message events (openclaw#67510)

* fix(bluebubbles): restore inbound image attachments and accept updated-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (openclaw#64105, openclaw#61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (openclaw#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (openclaw#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (openclaw#67437)

Closes openclaw#64105, closes openclaw#61861, closes openclaw#65430.

* fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests

- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
  allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
  instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
  asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
  success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.

* fix(ci): update raw-fetch allowlist line number after dispatcher strip

* fix(bluebubbles): resolve PR review findings (openclaw#67510)

- monitor-processing: move attachment retry into the !rawBody guard so
  image-only new-message events that arrive with empty attachments and
  empty text are recovered via a BB API refetch before being dropped.
  The existing retry block at the end of processMessageAfterDedupe was
  unreachable for this case because the !rawBody early-return fired
  first. (Greptile)
- monitor: derive isAttachmentUpdate from the normalized message shape
  instead of raw payload.data.attachments so updated-message webhooks
  with attachments under wrapper formats (payload.message, JSON-string
  payloads) are correctly routed through for processing instead of
  silently filtered. (Codex)
- types: use bundled-undici fetch when init.dispatcher is present so
  the SSRF guard's DNS-pinning dispatcher is preserved when this
  function is called as fetchImpl from guarded callers (e.g. the
  attachment download path via fetchRemoteMedia). Falls back to
  globalThis.fetch when no dispatcher is present so tests that stub
  globalThis.fetch keep working. (Codex)
- attachments: blueBubblesPolicy returns undefined for the non-private
  case (matching monitor-processing's helper) so sendBlueBubblesAttachment
  stops routing localhost BB through the SSRF guard. (Greptile)
- scripts/check-no-raw-channel-fetch: bump the types.ts allowlist line
  to match the restructured non-SSRF branch.

* fix(bluebubbles): move attachment retry before rawBody guard, fix stale log

Move the attachment retry block (2s BB API refetch for empty attachments)
before the !rawBody early-return guard. Previously, image-only messages
with text='' and attachments=[] would be dropped by the !rawBody check
before the retry could fire, making fix openclaw#4 dead code for its primary
use-case. Now the retry runs first and recomputes the placeholder from
resolved attachments so rawBody becomes non-empty when media is found.

Also fix stale log message that still said 'without reaction' after the
filter was expanded to pass through attachment updates.

* fix(bluebubbles): revert undici import, restore dispatcher-strip approach

Revert the @claude bot's undici import in types.ts — it introduced a
direct 'undici' dependency that is not declared in the BB extension's
package.json and would break isolated plugin installs. Restore the
original dispatcher-strip approach which is correct: the SSRF guard
already completed validation upstream before calling this function as
fetchImpl, so stripping the dispatcher does not weaken security.

* fix(bluebubbles): remove dead empty-body recovery block in !rawBody guard

The empty-body attachment-recovery block added in the earlier PR revision
is now redundant because the main retry block was moved above the rawBody
computation in 0d7d1c4. Worse, that leftover block reassigned the
(now-const) placeholder variable, throwing `TypeError: Assignment to
constant variable` at runtime for image-only messages — breaking the very
recovery path it was meant to protect (flagged by Codex on 4bfc2777).

Remove the dead block; the up-front retry already handles the image-only
case by recovering attachments before the rawBody computation, so once we
reach the !rawBody guard with an empty body it is genuinely empty and
should drop as before.

* fix(ci): update raw-fetch allowlist line after dispatcher-strip revert

279dba1 reverted types.ts back to the dispatcher-strip approach,
which put the `fetch(url, ...)` call at line 189 instead of line 198.
Bump the allowlist entry to match so `lint:tmp:no-raw-channel-fetch`
stops failing check-additional.

* test(pdf-tool): update stale opus-4-6 constant to opus-4-7

`628b454eff feat: default Anthropic to Opus 4.7` bumped the bundled
anthropic image default to `claude-opus-4-7` but missed updating the
`ANTHROPIC_PDF_MODEL` constant in pdf-tool.model-config.test.ts. The
tests now fail on any PR that runs the `checks-node-agentic-agents-plugins`
shard because the resolver returns 4-7 while the test asserts 4-6.

Bump the constant to 4-7 to match the bundled default.

---------

Co-authored-by: Lobster <10343873+omarshahine@users.noreply.github.com>
kvnkho pushed a commit to kvnkho/openclaw that referenced this pull request Apr 17, 2026
…d-message events (openclaw#67510)

* fix(bluebubbles): restore inbound image attachments and accept updated-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (openclaw#64105, openclaw#61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (openclaw#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (openclaw#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (openclaw#67437)

Closes openclaw#64105, closes openclaw#61861, closes openclaw#65430.

* fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests

- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
  allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
  instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
  asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
  success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.

* fix(ci): update raw-fetch allowlist line number after dispatcher strip

* fix(bluebubbles): resolve PR review findings (openclaw#67510)

- monitor-processing: move attachment retry into the !rawBody guard so
  image-only new-message events that arrive with empty attachments and
  empty text are recovered via a BB API refetch before being dropped.
  The existing retry block at the end of processMessageAfterDedupe was
  unreachable for this case because the !rawBody early-return fired
  first. (Greptile)
- monitor: derive isAttachmentUpdate from the normalized message shape
  instead of raw payload.data.attachments so updated-message webhooks
  with attachments under wrapper formats (payload.message, JSON-string
  payloads) are correctly routed through for processing instead of
  silently filtered. (Codex)
- types: use bundled-undici fetch when init.dispatcher is present so
  the SSRF guard's DNS-pinning dispatcher is preserved when this
  function is called as fetchImpl from guarded callers (e.g. the
  attachment download path via fetchRemoteMedia). Falls back to
  globalThis.fetch when no dispatcher is present so tests that stub
  globalThis.fetch keep working. (Codex)
- attachments: blueBubblesPolicy returns undefined for the non-private
  case (matching monitor-processing's helper) so sendBlueBubblesAttachment
  stops routing localhost BB through the SSRF guard. (Greptile)
- scripts/check-no-raw-channel-fetch: bump the types.ts allowlist line
  to match the restructured non-SSRF branch.

* fix(bluebubbles): move attachment retry before rawBody guard, fix stale log

Move the attachment retry block (2s BB API refetch for empty attachments)
before the !rawBody early-return guard. Previously, image-only messages
with text='' and attachments=[] would be dropped by the !rawBody check
before the retry could fire, making fix openclaw#4 dead code for its primary
use-case. Now the retry runs first and recomputes the placeholder from
resolved attachments so rawBody becomes non-empty when media is found.

Also fix stale log message that still said 'without reaction' after the
filter was expanded to pass through attachment updates.

* fix(bluebubbles): revert undici import, restore dispatcher-strip approach

Revert the @claude bot's undici import in types.ts — it introduced a
direct 'undici' dependency that is not declared in the BB extension's
package.json and would break isolated plugin installs. Restore the
original dispatcher-strip approach which is correct: the SSRF guard
already completed validation upstream before calling this function as
fetchImpl, so stripping the dispatcher does not weaken security.

* fix(bluebubbles): remove dead empty-body recovery block in !rawBody guard

The empty-body attachment-recovery block added in the earlier PR revision
is now redundant because the main retry block was moved above the rawBody
computation in 0d7d1c4. Worse, that leftover block reassigned the
(now-const) placeholder variable, throwing `TypeError: Assignment to
constant variable` at runtime for image-only messages — breaking the very
recovery path it was meant to protect (flagged by Codex on 4bfc2777).

Remove the dead block; the up-front retry already handles the image-only
case by recovering attachments before the rawBody computation, so once we
reach the !rawBody guard with an empty body it is genuinely empty and
should drop as before.

* fix(ci): update raw-fetch allowlist line after dispatcher-strip revert

279dba1 reverted types.ts back to the dispatcher-strip approach,
which put the `fetch(url, ...)` call at line 189 instead of line 198.
Bump the allowlist entry to match so `lint:tmp:no-raw-channel-fetch`
stops failing check-additional.

* test(pdf-tool): update stale opus-4-6 constant to opus-4-7

`628b454eff feat: default Anthropic to Opus 4.7` bumped the bundled
anthropic image default to `claude-opus-4-7` but missed updating the
`ANTHROPIC_PDF_MODEL` constant in pdf-tool.model-config.test.ts. The
tests now fail on any PR that runs the `checks-node-agentic-agents-plugins`
shard because the resolver returns 4-7 while the test asserts 4-6.

Bump the constant to 4-7 to match the bundled default.

---------

Co-authored-by: Lobster <10343873+omarshahine@users.noreply.github.com>
@omarshahine
Copy link
Copy Markdown
Contributor

Thanks for carrying this forward from the auto-closed #52239 @aidenwildenbadt. Closing as superseded — the inbound webhook dedupe, replay, and updated-message/edit handling territory has been extensively reworked in the following landed PRs:

At 4,699 additions / 1,077 deletions, a rebase would be a near-rewrite against the current shape. If you still see a specific gap (edit-aware dedupe keys, reply-context handling) that isn't covered in the landed sequence, please open a focused issue with a repro and we can pull just that slice in. Thanks for the contribution.

Mquarmoc pushed a commit to Mquarmoc/openclaw that referenced this pull request Apr 20, 2026
…d-message events (openclaw#67510)

* fix(bluebubbles): restore inbound image attachments and accept updated-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (openclaw#64105, openclaw#61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (openclaw#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (openclaw#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (openclaw#67437)

Closes openclaw#64105, closes openclaw#61861, closes openclaw#65430.

* fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests

- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
  allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
  instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
  asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
  success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.

* fix(ci): update raw-fetch allowlist line number after dispatcher strip

* fix(bluebubbles): resolve PR review findings (openclaw#67510)

- monitor-processing: move attachment retry into the !rawBody guard so
  image-only new-message events that arrive with empty attachments and
  empty text are recovered via a BB API refetch before being dropped.
  The existing retry block at the end of processMessageAfterDedupe was
  unreachable for this case because the !rawBody early-return fired
  first. (Greptile)
- monitor: derive isAttachmentUpdate from the normalized message shape
  instead of raw payload.data.attachments so updated-message webhooks
  with attachments under wrapper formats (payload.message, JSON-string
  payloads) are correctly routed through for processing instead of
  silently filtered. (Codex)
- types: use bundled-undici fetch when init.dispatcher is present so
  the SSRF guard's DNS-pinning dispatcher is preserved when this
  function is called as fetchImpl from guarded callers (e.g. the
  attachment download path via fetchRemoteMedia). Falls back to
  globalThis.fetch when no dispatcher is present so tests that stub
  globalThis.fetch keep working. (Codex)
- attachments: blueBubblesPolicy returns undefined for the non-private
  case (matching monitor-processing's helper) so sendBlueBubblesAttachment
  stops routing localhost BB through the SSRF guard. (Greptile)
- scripts/check-no-raw-channel-fetch: bump the types.ts allowlist line
  to match the restructured non-SSRF branch.

* fix(bluebubbles): move attachment retry before rawBody guard, fix stale log

Move the attachment retry block (2s BB API refetch for empty attachments)
before the !rawBody early-return guard. Previously, image-only messages
with text='' and attachments=[] would be dropped by the !rawBody check
before the retry could fire, making fix openclaw#4 dead code for its primary
use-case. Now the retry runs first and recomputes the placeholder from
resolved attachments so rawBody becomes non-empty when media is found.

Also fix stale log message that still said 'without reaction' after the
filter was expanded to pass through attachment updates.

* fix(bluebubbles): revert undici import, restore dispatcher-strip approach

Revert the @claude bot's undici import in types.ts — it introduced a
direct 'undici' dependency that is not declared in the BB extension's
package.json and would break isolated plugin installs. Restore the
original dispatcher-strip approach which is correct: the SSRF guard
already completed validation upstream before calling this function as
fetchImpl, so stripping the dispatcher does not weaken security.

* fix(bluebubbles): remove dead empty-body recovery block in !rawBody guard

The empty-body attachment-recovery block added in the earlier PR revision
is now redundant because the main retry block was moved above the rawBody
computation in 0d7d1c4. Worse, that leftover block reassigned the
(now-const) placeholder variable, throwing `TypeError: Assignment to
constant variable` at runtime for image-only messages — breaking the very
recovery path it was meant to protect (flagged by Codex on 4bfc2777).

Remove the dead block; the up-front retry already handles the image-only
case by recovering attachments before the rawBody computation, so once we
reach the !rawBody guard with an empty body it is genuinely empty and
should drop as before.

* fix(ci): update raw-fetch allowlist line after dispatcher-strip revert

279dba1 reverted types.ts back to the dispatcher-strip approach,
which put the `fetch(url, ...)` call at line 189 instead of line 198.
Bump the allowlist entry to match so `lint:tmp:no-raw-channel-fetch`
stops failing check-additional.

* test(pdf-tool): update stale opus-4-6 constant to opus-4-7

`628b454eff feat: default Anthropic to Opus 4.7` bumped the bundled
anthropic image default to `claude-opus-4-7` but missed updating the
`ANTHROPIC_PDF_MODEL` constant in pdf-tool.model-config.test.ts. The
tests now fail on any PR that runs the `checks-node-agentic-agents-plugins`
shard because the resolver returns 4-7 while the test asserts 4-6.

Bump the constant to 4-7 to match the bundled default.

---------

Co-authored-by: Lobster <10343873+omarshahine@users.noreply.github.com>
lovewanwan pushed a commit to lovewanwan/openclaw that referenced this pull request Apr 28, 2026
…d-message events (openclaw#67510)

* fix(bluebubbles): restore inbound image attachments and accept updated-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (openclaw#64105, openclaw#61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (openclaw#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (openclaw#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (openclaw#67437)

Closes openclaw#64105, closes openclaw#61861, closes openclaw#65430.

* fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests

- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
  allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
  instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
  asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
  success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.

* fix(ci): update raw-fetch allowlist line number after dispatcher strip

* fix(bluebubbles): resolve PR review findings (openclaw#67510)

- monitor-processing: move attachment retry into the !rawBody guard so
  image-only new-message events that arrive with empty attachments and
  empty text are recovered via a BB API refetch before being dropped.
  The existing retry block at the end of processMessageAfterDedupe was
  unreachable for this case because the !rawBody early-return fired
  first. (Greptile)
- monitor: derive isAttachmentUpdate from the normalized message shape
  instead of raw payload.data.attachments so updated-message webhooks
  with attachments under wrapper formats (payload.message, JSON-string
  payloads) are correctly routed through for processing instead of
  silently filtered. (Codex)
- types: use bundled-undici fetch when init.dispatcher is present so
  the SSRF guard's DNS-pinning dispatcher is preserved when this
  function is called as fetchImpl from guarded callers (e.g. the
  attachment download path via fetchRemoteMedia). Falls back to
  globalThis.fetch when no dispatcher is present so tests that stub
  globalThis.fetch keep working. (Codex)
- attachments: blueBubblesPolicy returns undefined for the non-private
  case (matching monitor-processing's helper) so sendBlueBubblesAttachment
  stops routing localhost BB through the SSRF guard. (Greptile)
- scripts/check-no-raw-channel-fetch: bump the types.ts allowlist line
  to match the restructured non-SSRF branch.

* fix(bluebubbles): move attachment retry before rawBody guard, fix stale log

Move the attachment retry block (2s BB API refetch for empty attachments)
before the !rawBody early-return guard. Previously, image-only messages
with text='' and attachments=[] would be dropped by the !rawBody check
before the retry could fire, making fix openclaw#4 dead code for its primary
use-case. Now the retry runs first and recomputes the placeholder from
resolved attachments so rawBody becomes non-empty when media is found.

Also fix stale log message that still said 'without reaction' after the
filter was expanded to pass through attachment updates.

* fix(bluebubbles): revert undici import, restore dispatcher-strip approach

Revert the @claude bot's undici import in types.ts — it introduced a
direct 'undici' dependency that is not declared in the BB extension's
package.json and would break isolated plugin installs. Restore the
original dispatcher-strip approach which is correct: the SSRF guard
already completed validation upstream before calling this function as
fetchImpl, so stripping the dispatcher does not weaken security.

* fix(bluebubbles): remove dead empty-body recovery block in !rawBody guard

The empty-body attachment-recovery block added in the earlier PR revision
is now redundant because the main retry block was moved above the rawBody
computation in 0d7d1c4. Worse, that leftover block reassigned the
(now-const) placeholder variable, throwing `TypeError: Assignment to
constant variable` at runtime for image-only messages — breaking the very
recovery path it was meant to protect (flagged by Codex on 4bfc2777).

Remove the dead block; the up-front retry already handles the image-only
case by recovering attachments before the rawBody computation, so once we
reach the !rawBody guard with an empty body it is genuinely empty and
should drop as before.

* fix(ci): update raw-fetch allowlist line after dispatcher-strip revert

279dba1 reverted types.ts back to the dispatcher-strip approach,
which put the `fetch(url, ...)` call at line 189 instead of line 198.
Bump the allowlist entry to match so `lint:tmp:no-raw-channel-fetch`
stops failing check-additional.

* test(pdf-tool): update stale opus-4-6 constant to opus-4-7

`b539120bf0 feat: default Anthropic to Opus 4.7` bumped the bundled
anthropic image default to `claude-opus-4-7` but missed updating the
`ANTHROPIC_PDF_MODEL` constant in pdf-tool.model-config.test.ts. The
tests now fail on any PR that runs the `checks-node-agentic-agents-plugins`
shard because the resolver returns 4-7 while the test asserts 4-6.

Bump the constant to 4-7 to match the bundled default.

---------

Co-authored-by: Lobster <10343873+omarshahine@users.noreply.github.com>
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
…d-message events (openclaw#67510)

* fix(bluebubbles): restore inbound image attachments and accept updated-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (openclaw#64105, openclaw#61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (openclaw#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (openclaw#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (openclaw#67437)

Closes openclaw#64105, closes openclaw#61861, closes openclaw#65430.

* fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests

- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
  allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
  instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
  asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
  success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.

* fix(ci): update raw-fetch allowlist line number after dispatcher strip

* fix(bluebubbles): resolve PR review findings (openclaw#67510)

- monitor-processing: move attachment retry into the !rawBody guard so
  image-only new-message events that arrive with empty attachments and
  empty text are recovered via a BB API refetch before being dropped.
  The existing retry block at the end of processMessageAfterDedupe was
  unreachable for this case because the !rawBody early-return fired
  first. (Greptile)
- monitor: derive isAttachmentUpdate from the normalized message shape
  instead of raw payload.data.attachments so updated-message webhooks
  with attachments under wrapper formats (payload.message, JSON-string
  payloads) are correctly routed through for processing instead of
  silently filtered. (Codex)
- types: use bundled-undici fetch when init.dispatcher is present so
  the SSRF guard's DNS-pinning dispatcher is preserved when this
  function is called as fetchImpl from guarded callers (e.g. the
  attachment download path via fetchRemoteMedia). Falls back to
  globalThis.fetch when no dispatcher is present so tests that stub
  globalThis.fetch keep working. (Codex)
- attachments: blueBubblesPolicy returns undefined for the non-private
  case (matching monitor-processing's helper) so sendBlueBubblesAttachment
  stops routing localhost BB through the SSRF guard. (Greptile)
- scripts/check-no-raw-channel-fetch: bump the types.ts allowlist line
  to match the restructured non-SSRF branch.

* fix(bluebubbles): move attachment retry before rawBody guard, fix stale log

Move the attachment retry block (2s BB API refetch for empty attachments)
before the !rawBody early-return guard. Previously, image-only messages
with text='' and attachments=[] would be dropped by the !rawBody check
before the retry could fire, making fix openclaw#4 dead code for its primary
use-case. Now the retry runs first and recomputes the placeholder from
resolved attachments so rawBody becomes non-empty when media is found.

Also fix stale log message that still said 'without reaction' after the
filter was expanded to pass through attachment updates.

* fix(bluebubbles): revert undici import, restore dispatcher-strip approach

Revert the @claude bot's undici import in types.ts — it introduced a
direct 'undici' dependency that is not declared in the BB extension's
package.json and would break isolated plugin installs. Restore the
original dispatcher-strip approach which is correct: the SSRF guard
already completed validation upstream before calling this function as
fetchImpl, so stripping the dispatcher does not weaken security.

* fix(bluebubbles): remove dead empty-body recovery block in !rawBody guard

The empty-body attachment-recovery block added in the earlier PR revision
is now redundant because the main retry block was moved above the rawBody
computation in 0d7d1c4. Worse, that leftover block reassigned the
(now-const) placeholder variable, throwing `TypeError: Assignment to
constant variable` at runtime for image-only messages — breaking the very
recovery path it was meant to protect (flagged by Codex on 4bfc2777).

Remove the dead block; the up-front retry already handles the image-only
case by recovering attachments before the rawBody computation, so once we
reach the !rawBody guard with an empty body it is genuinely empty and
should drop as before.

* fix(ci): update raw-fetch allowlist line after dispatcher-strip revert

279dba1 reverted types.ts back to the dispatcher-strip approach,
which put the `fetch(url, ...)` call at line 189 instead of line 198.
Bump the allowlist entry to match so `lint:tmp:no-raw-channel-fetch`
stops failing check-additional.

* test(pdf-tool): update stale opus-4-6 constant to opus-4-7

`3e103fc4d4 feat: default Anthropic to Opus 4.7` bumped the bundled
anthropic image default to `claude-opus-4-7` but missed updating the
`ANTHROPIC_PDF_MODEL` constant in pdf-tool.model-config.test.ts. The
tests now fail on any PR that runs the `checks-node-agentic-agents-plugins`
shard because the resolver returns 4-7 while the test asserts 4-6.

Bump the constant to 4-7 to match the bundled default.

---------

Co-authored-by: Lobster <10343873+omarshahine@users.noreply.github.com>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
…d-message events (openclaw#67510)

* fix(bluebubbles): restore inbound image attachments and accept updated-message events

Four interconnected fixes for BlueBubbles inbound media:

1. Strip bundled-undici dispatcher from non-SSRF fetch path so attachment
   downloads no longer silently fail on Node 22+ (openclaw#64105, openclaw#61861)

2. Accept updated-message webhook events that carry attachments instead of
   filtering them as non-reaction events (openclaw#65430)

3. Include eventType in the persistent GUID dedup key so updated-message
   follow-ups are not rejected as duplicates of the original new-message (openclaw#52277)

4. Retry attachment fetch from BB API (2s delay) when the initial webhook
   arrives with an empty attachments array — image-only messages and
   updated-message events only (openclaw#67437)

Closes openclaw#64105, closes openclaw#61861, closes openclaw#65430.

* fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests

- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
  allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
  instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
  asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
  success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.

* fix(ci): update raw-fetch allowlist line number after dispatcher strip

* fix(bluebubbles): resolve PR review findings (openclaw#67510)

- monitor-processing: move attachment retry into the !rawBody guard so
  image-only new-message events that arrive with empty attachments and
  empty text are recovered via a BB API refetch before being dropped.
  The existing retry block at the end of processMessageAfterDedupe was
  unreachable for this case because the !rawBody early-return fired
  first. (Greptile)
- monitor: derive isAttachmentUpdate from the normalized message shape
  instead of raw payload.data.attachments so updated-message webhooks
  with attachments under wrapper formats (payload.message, JSON-string
  payloads) are correctly routed through for processing instead of
  silently filtered. (Codex)
- types: use bundled-undici fetch when init.dispatcher is present so
  the SSRF guard's DNS-pinning dispatcher is preserved when this
  function is called as fetchImpl from guarded callers (e.g. the
  attachment download path via fetchRemoteMedia). Falls back to
  globalThis.fetch when no dispatcher is present so tests that stub
  globalThis.fetch keep working. (Codex)
- attachments: blueBubblesPolicy returns undefined for the non-private
  case (matching monitor-processing's helper) so sendBlueBubblesAttachment
  stops routing localhost BB through the SSRF guard. (Greptile)
- scripts/check-no-raw-channel-fetch: bump the types.ts allowlist line
  to match the restructured non-SSRF branch.

* fix(bluebubbles): move attachment retry before rawBody guard, fix stale log

Move the attachment retry block (2s BB API refetch for empty attachments)
before the !rawBody early-return guard. Previously, image-only messages
with text='' and attachments=[] would be dropped by the !rawBody check
before the retry could fire, making fix openclaw#4 dead code for its primary
use-case. Now the retry runs first and recomputes the placeholder from
resolved attachments so rawBody becomes non-empty when media is found.

Also fix stale log message that still said 'without reaction' after the
filter was expanded to pass through attachment updates.

* fix(bluebubbles): revert undici import, restore dispatcher-strip approach

Revert the @claude bot's undici import in types.ts — it introduced a
direct 'undici' dependency that is not declared in the BB extension's
package.json and would break isolated plugin installs. Restore the
original dispatcher-strip approach which is correct: the SSRF guard
already completed validation upstream before calling this function as
fetchImpl, so stripping the dispatcher does not weaken security.

* fix(bluebubbles): remove dead empty-body recovery block in !rawBody guard

The empty-body attachment-recovery block added in the earlier PR revision
is now redundant because the main retry block was moved above the rawBody
computation in 0d7d1c4. Worse, that leftover block reassigned the
(now-const) placeholder variable, throwing `TypeError: Assignment to
constant variable` at runtime for image-only messages — breaking the very
recovery path it was meant to protect (flagged by Codex on 4bfc2777).

Remove the dead block; the up-front retry already handles the image-only
case by recovering attachments before the rawBody computation, so once we
reach the !rawBody guard with an empty body it is genuinely empty and
should drop as before.

* fix(ci): update raw-fetch allowlist line after dispatcher-strip revert

279dba1 reverted types.ts back to the dispatcher-strip approach,
which put the `fetch(url, ...)` call at line 189 instead of line 198.
Bump the allowlist entry to match so `lint:tmp:no-raw-channel-fetch`
stops failing check-additional.

* test(pdf-tool): update stale opus-4-6 constant to opus-4-7

`98a49ab365 feat: default Anthropic to Opus 4.7` bumped the bundled
anthropic image default to `claude-opus-4-7` but missed updating the
`ANTHROPIC_PDF_MODEL` constant in pdf-tool.model-config.test.ts. The
tests now fail on any PR that runs the `checks-node-agentic-agents-plugins`
shard because the resolver returns 4-7 while the test asserts 4-6.

Bump the constant to 4-7 to match the bundled default.

---------

Co-authored-by: Lobster <10343873+omarshahine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: bluebubbles Channel integration: bluebubbles size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants