Skip to content

fix(simplex): make websocket handling reliable - #26480

Open
gedigi wants to merge 1 commit into
NousResearch:mainfrom
gedigi:fix/simplex-inbound-polling
Open

fix(simplex): make websocket handling reliable#26480
gedigi wants to merge 1 commit into
NousResearch:mainfrom
gedigi:fix/simplex-inbound-polling

Conversation

@gedigi

@gedigi gedigi commented May 15, 2026

Copy link
Copy Markdown

Summary

  • normalize wrapped and nested SimpleX WebSocket event payloads before processing inbound messages
  • keep the persistent WebSocket as the primary stream, with one catch-up pass after reconnect instead of continuous polling
  • de-duplicate live stream items against catch-up tail responses and make outbound send failures report failure instead of fake success
  • quote SimpleX command targets, prefer local display names for delivery, and allow direct SimpleX send targets
  • update SimpleX docs and regression tests for the real WebSocket/send behavior

Test plan

  • python -m py_compile plugins/platforms/simplex/adapter.py
  • python -m pytest tests/gateway/test_simplex_plugin.py tests/tools/test_send_message_tool.py -q -o 'addopts='
  • git diff --check

Local validation

  • Restarted the local Hermes gateway
  • Verified SimpleX gateway connects to the configured local WebSocket after restart

@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have type/bug Something isn't working comp/plugins Plugin system and bundled plugins comp/gateway Gateway runner, session dispatch, delivery labels May 15, 2026
@brandon-btcgroup

Copy link
Copy Markdown

fwiw on the group-send line here (the websocket-reliability work is separate —
not commenting on that): this PR sends to groups via the name-based
#<display_name> form. I didn't run this branch, so this is just a data point
from an independent implementation, not a test of your code.

In my own deployment I went with the numeric-id structured form instead —
/_send #<groupId> json [{"msgContent": {"type": "text", "text": …}}] — for
two reasons that held up live:

  • Group display names aren't guaranteed unique, so #<name> can resolve to the
    wrong group or fail to resolve; the numeric groupId (already in hand from
    the inbound event) is unambiguous. feat(simplex): add SimpleX Chat platform adapter #4666 makes the same argument.
  • The json payload escapes newlines/backslashes; the plain command body
    truncates a multi-line reply at the first newline. A 3-line haiku to a group
    arrived intact with the json form and was cut to one line with the plain
    form.

Sharing in case it's useful for the send path; the reconnect/reliability
changes themselves look orthogonal to this.

@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.

Thank you for the reliability work. Current main still has a concrete nested singular-event gap: _handle_event passes newChatItem directly to _handle_chat_item (plugins/platforms/simplex/adapter.py:434), while that handler reads chatInfo only at the top level (plugins/platforms/simplex/adapter.py:472-477).

Problems

  • The group-send hunk at plugins/platforms/simplex/adapter.py:798 changes stable numeric group routing to a display-name command. Current main intentionally uses /_send #<id> json ... (plugins/platforms/simplex/adapter.py:833-843), and its regression test records why: display-name lookup can silently drop messages and JSON preserves multiline content (tests/gateway/test_simplex_plugin.py:231-253). This also matches the supplied review comment.
  • The adapter has been substantially refactored since this PR: current main uses _handle_chat_item and _send_command correlation handling (plugins/platforms/simplex/adapter.py:370-475, 758-795). The patch needs selective integration rather than direct application.

Suggested changes

  • Port only the nested newChatItem normalization into the current dispatch path and add its regression test.
  • Preserve numeric group:<id> identities and structured group sends.

Automated hermes-sweeper review.

Comment thread plugins/platforms/simplex/adapter.py Outdated
@@ -505,17 +796,19 @@ async def send(
corr_id = self._make_corr_id()

if chat_id.startswith("group:"):

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.

Please retain numeric group IDs and the structured /_send #<id> json ... form here. Current main documents and tests that name-based group commands can resolve ambiguously or silently drop, while the JSON form also preserves multiline content (tests/gateway/test_simplex_plugin.py:231-253).

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Singular newChatItem events (and some newChatItems array elements) nest
the AChatItem one level down ({type: newChatItem, chatItem: {chatInfo,
chatItem}}), but _handle_chat_item only reads chatInfo/chatItem at the
top level, so those messages were silently dropped.

Port only the nested-wrapper normalization from the original branch into
the current dispatch, per review: _normalize_chat_item_wrapper unwraps
the nested AChatItem form, maps the {chatInfo, item} field-name variant,
and passes already-normalized wrappers through untouched. Applied in
_handle_event for both event shapes and idempotently at the top of
_handle_chat_item so the deferred rcvFileComplete replay path is covered
too.

Group routing is unchanged: numeric group IDs with the structured
/_send #<id> json form (display-name commands can resolve ambiguously
or silently drop, and plain commands truncate multiline content).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LJcz6L6JJWpQv5FrTBz6J1
@gedigi
gedigi force-pushed the fix/simplex-inbound-polling branch from e555d8b to c92e84e Compare July 13, 2026 11:35
@gedigi

gedigi commented Jul 13, 2026

Copy link
Copy Markdown
Author

@teknium1 Rewrote the branch on current main per your review (force-pushed as a single commit):

  • Group routing untouched — numeric group IDs with the structured /_send #<id> json ... form stay exactly as on main, so nothing regresses on ambiguous display-name resolution or multiline content (test_send_group still passes unchanged).
  • Ported only the nested normalization into the current dispatch_normalize_chat_item_wrapper() unwraps the nested {type: newChatItem, chatItem: {chatInfo, chatItem}} shape, maps the {chatInfo, item} field-name variant, and passes already-normalized wrappers through. Applied in _handle_event for both newChatItem and newChatItems, and idempotently at the top of _handle_chat_item so the deferred rcvFileComplete replay path is covered too.
  • Added regression tests for the nested variants (4 unit + 2 end-to-end through _handle_event).

Everything else from the original branch (catch-up, dedupe, health-check ping, ephemeral send fallback, display-name addressing) is dropped as superseded by the adapter refactor on main.

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 comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants