Skip to content

fix(gateway): harden BlueBubbles delivery and acknowledgments - #18395

Open
benjaminsehl wants to merge 2 commits into
NousResearch:mainfrom
benjaminsehl:fix/bluebubbles-webhook-dedupe
Open

fix(gateway): harden BlueBubbles delivery and acknowledgments#18395
benjaminsehl wants to merge 2 commits into
NousResearch:mainfrom
benjaminsehl:fix/bluebubbles-webhook-dedupe

Conversation

@benjaminsehl

@benjaminsehl benjaminsehl commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

BlueBubbles can deliver one iMessage as both new-message and updated-message, including overlapping deliveries and later attachment enrichment. This replaces the original persistent-hash approach with bounded in-memory reservation semantics and adds a contextual, turn-local quick acknowledgment for substantive iMessages.

Duplicate-delivery protection

  • Canonicalizes equivalent local webhook callbacks and keeps exactly one registration subscribed to both message events.
  • Reserves a validated stable message GUID before attachment I/O.
  • Makes overlapping duplicate deliveries join one owner outcome, with bounded waiters, attempts, request-wide deadline, cache size, TTL, and attachment count.
  • Allows takeover after owner failure and returns retryable HTTP 503 under bounded capacity/deadline pressure instead of dispatching duplicate work.
  • Preserves attachment order and dispatches genuinely late media once as attachment-only enrichment.
  • Rolls reservations back on download, setup, scheduling, dispatch, cancellation, and enrichment failures so BlueBubbles retries remain possible.
  • Retains metadata only; raw webhook payloads are not stored.

Contextual quick acknowledgment

  • Optional and BlueBubbles-only; skips slash commands and trivial replies.
  • Uses one hard generation/send deadline with reserved fallback budget.
  • Accepts only strict pending-work language and never allows acknowledgment failure to abort the main turn.
  • Passes visible acknowledgment context into only the exact current agent invocation without mutating conversation history or introducing a synthetic user role.
  • Treats empty-message ReadTimeout as an ambiguous send so an already-delivered message does not trigger a duplicate fallback bubble.

Verification

  • 192 focused gateway tests pass.
  • Ruff, compileall, and git diff --check pass.
  • The complete production diff received independent security/logic review after each blocking finding was fixed.
  • Clean patch application against current main passes the same 192-test gate.
  • Live BlueBubbles verification confirmed one inbound agent run, one acknowledgment, one final response, and exactly one canonical webhook registration.

Test command

python -m pytest \
  tests/gateway/test_bluebubbles.py \
  tests/gateway/test_prompt_tail_freeze.py \
  tests/gateway/test_run_progress_topics.py \
  tests/gateway/test_aiohttp_body_caps.py \
  -o addopts=

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused BlueBubbles hardening. Current main still subscribes to both events at gateway/platforms/bluebubbles.py:374-377 and dispatches each accepted webhook at :1040, so inbound deduplication remains needed.

Problems

  • gateway/platforms/bluebubbles.py:885-895 includes chatGuid in the key but does not use chatIdentifier. The linked #30708 report documents same-GUID deliveries with any;-;+address on one event and only a bare chat identifier on another. Those forms become different keys, so both deliveries can reach the new guard at :1050 and dispatch.

Suggested changes

  • Use a GUID-first key, or canonicalize the fallback chat forms, and add an async two-webhook regression covering those differing payload shapes.
  • Preserve main's existing log-safe registration URL property at current gateway/platforms/bluebubbles.py:334-340 while salvaging the branch.

Automated hermes-sweeper review.

Comment thread gateway/platforms/bluebubbles.py Outdated
chats = record.get("chats") or []
if chats and isinstance(chats[0], dict):
chat_guid = self._value(chats[0].get("guid"), chats[0].get("chatGuid"))
return f"{chat_guid or 'unknown-chat'}:{msg_guid}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This key changes when duplicate BlueBubbles events expose the same message GUID under different chat forms. The linked #30708 report shows any;-;+address on one event and only a bare chat identifier on the other; because chatIdentifier is not considered here, they become different keys and both dispatch. Please key GUID-first or canonicalize the fallback identifiers, with an end-to-end two-webhook regression.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
JoshHobbs added a commit to JoshHobbs/hermes-agent that referenced this pull request Jul 18, 2026
BlueBubbles surfaces a single 1:1 conversation under more than one chat_id,
and build_session_key used the raw value, so one thread split across several
session keys:

  1. The adapter sets `session_chat_id = chat_guid or chat_identifier`
     (gateway/platforms/bluebubbles.py), so a webhook carrying no chat GUID
     falls back to the bare handle. The two forms key differently:
     `any;-;+1555…` vs `+1555…`.
  2. The GUID form recorded for one conversation is not stable over time. On
     the deployment this was found on, sessions carry `iMessage;-;+1555…` from
     May and `any;-;+1555…` since July, while the server today reports exactly
     one chat for that handle (`any;-;+1555…`, chatIdentifier `+1555…`) and
     uses the `any` prefix for every chat it knows about. Whatever drove that
     change server-side, the routing key should not depend on it.

The usual report of this is duplicate replies (NousResearch#30708, NousResearch#34372): two chat-id
variants defeat the in-flight guard, so a message gets answered twice. The
split has a second and worse consequence that has not been reported. Each
variant is a separate SessionEntry with its own updated_at, so a variant that
has not been messaged recently goes stale while the conversation continues
under another. When a webhook eventually routes to the stale variant,
_should_reset() finds it idle and clears an actively-used conversation.
Observed in production: a thread whose live session held 298 messages was
reset because a GUID-less webhook landed on a sibling key last touched 20 days
earlier. The notice reads "inactive for 3h" because it renders
policy.idle_minutes rather than measured elapsed time, so it does not point at
the real cause.

Canonicalize the DM chat_id the way WhatsApp already canonicalizes JID/LID
aliases: unwrap the `<service>;-;` prefix so every form of one conversation
maps to the bare handle. Group GUIDs use `;+;` and carry an opaque chat id
rather than a participant handle, so they are returned untouched, as is every
other platform. BlueBubbles needs no group-participant equivalent of the
WhatsApp fix: the adapter already sets user_id from handle.address, which is
a bare handle.

Where a deployment does have distinct iMessage and SMS chats for the same
handle, those now share one session key. That is intended — one human, one
agent conversation — and replies to an inbound message are unaffected, since
they route on the live event's source.chat_id rather than on the key.

Existing sessions are not orphaned. Canonicalization rewrites only the routing
key, never source.chat_id, so when the exact-key lookup misses after upgrade,
find_latest_gateway_session_for_peer's peer-tuple fallback still matches the
stored row on (source, user_id, chat_id, chat_type, thread_id) and adopts the
transcript under the new key. The regression test drives build_session_key
rather than hardcoding the key, so it fails both if the canonicalization is
dropped and if source.chat_id is ever canonicalized too.

This is the session-key half of NousResearch#30708, complementary to the open adapter-side
PRs (NousResearch#45717, NousResearch#34378, NousResearch#18395, NousResearch#19976, NousResearch#27985) that suppress the duplicate-event
trigger. Those do not make the key stable on their own: the form drift in (2)
puts one conversation under two keys with no duplicate event involved, so the
reset stays reachable with any of them merged.
@benjaminsehl benjaminsehl changed the title fix: dedupe BlueBubbles webhook deliveries fix(gateway): harden BlueBubbles delivery and acknowledgments Jul 19, 2026
@benjaminsehl
benjaminsehl force-pushed the fix/bluebubbles-webhook-dedupe branch from d25ac89 to c3c8647 Compare July 19, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants