Fix(gateway): 3 bugs blocking bidirectional simpleX messaging - #26433
Fix(gateway): 3 bugs blocking bidirectional simpleX messaging#26433ruangraung wants to merge 6 commits into
Conversation
…placeholder credentials (closes NousResearch#22342, NousResearch#22763) (NousResearch#26320) * fix(langfuse): reject placeholder credentials with one-shot warning When operators leave HERMES_LANGFUSE_PUBLIC_KEY / HERMES_LANGFUSE_SECRET_KEY at a template value like 'placeholder', 'test-key', or 'your-langfuse-key', the Langfuse SDK silently accepts the credentials at construction time and drops every trace at flush time. No warning, no error — just an empty Langfuse dashboard the operator only notices hours later. Add prefix-based validation in _get_langfuse() against the documented 'pk-lf-' / 'sk-lf-' prefixes that Langfuse always issues server-side. Anything else fires a single warning naming the offending env var(s) with a log-safe value preview (full string for short placeholders so the operator knows which template they left in place; truncated for long values so a real secret pasted into the wrong field never hits the log), then short-circuits via the existing _INIT_FAILED cache so the warning fires once per process, not once per hook invocation. The check sits after the 'Langfuse is None' SDK-installed guard so hosts without the optional langfuse SDK don't see misleading 'set real keys' hints when the actionable fix is 'pip install langfuse'. Missing credentials remains the documented opt-out path and stays silent — no log noise for unconfigured installs. Fixes NousResearch#22763 Fixes NousResearch#23823 * fix(langfuse): use actual API request messages for generation input on_pre_llm_request previously used the messages kwarg alone, which could be None when Hermes passes the payload via request_messages, conversation_history, or user_message instead. Add _coerce_request_messages to pick the first available list across all variants, falling back to a synthetic user message. Generations now show the real outbound payload rather than an empty input. * fix(langfuse): record tool call outputs in traces Tool observations showed input (arguments) but output was always undefined. Root cause: when tool_call_id is empty, pre_tool_call stored observations under a unique time-based key that post_tool_call could never reconstruct, so every tool span was closed without output by the _finish_trace sweep. Fix pre/post matching by routing empty-tool_call_id tools through a per-name FIFO queue (pending_tools_by_name) instead of the time-based key. Tools with a tool_call_id continue to use the id-keyed dict. Also: - Preserve OpenAI-style nested function shape in serialized tool calls so Langfuse renders name/arguments correctly - Keep name + tool_call_id on role:tool messages for proper pairing - Backfill tool results onto the matching turn_tool_calls entry so the generation's tool-call record carries the result alongside arguments - Coerce request messages from whichever field the runtime provides (request_messages, messages, conversation_history, user_message) * fix(langfuse): salvage-review polish — drop dead is_first_turn, shallow-copy request_messages, real threaded FIFO test Self-review of the combined NousResearch#22345 + NousResearch#23831 salvage surfaced three issues worth fixing in the same PR rather than as follow-ups: 1. Drop is_first_turn from the pre_api_request hook. The boolean expression `not bool(conversation_history)` was wrong: conversation_history is reassigned to None mid-run after compression (5 sites in run_agent.py), so the value flips False -> True mid-conversation on every post-compression API call. The langfuse plugin never consumed it, so the kwarg was both misleading AND dead. 2. Replace copy.deepcopy(request_messages) with shallow list() copy. The pre_api_request hook contract discards return values (invoke_hook never writes back to api_kwargs), and the langfuse plugin's _serialize_messages already builds its own snapshot dicts via _safe_value. A deepcopy on every API call would walk every tool result and base64 image — significant overhead for no real isolation benefit. Shallow copy of the outer list protects against later mutations of api_messages without paying for the inner-dict walk. 3. Rename test_empty_tool_call_id_concurrent_fifo_order -> test_empty_tool_call_id_observations_are_fifo_within_tool_name and add a real test_threaded_post_calls_preserve_fifo_under_lock that spawns 8 threads behind a barrier to actually exercise _STATE_LOCK on the pending_tools_by_name queue. The original test was sequential and only validated Python list semantics; this one validates the lock discipline. 4. Fix stale 'Cleared by reset_cache_for_tests()' comment on _INIT_FAILED — that function does not exist. Tests reload the module via sys.modules.pop + importlib.import_module instead. Tests: 37 langfuse plugin tests pass, 658 plugin tests overall pass. --------- Co-authored-by: xxxigm <tuancanhnguyen706@gmail.com> Co-authored-by: Brian Conklin <brian@dralth.com>
…sResearch#26071) (NousResearch#26327) * feat(process-registry): add format_process_notification shared helper * feat(process-registry): add drain_notifications method * refactor(cli): use shared drain_notifications and format_process_notification * feat(tui): add background notification poller for completion_queue * feat(tui): wire notification poller into session init/finalize * refactor(tui): add post-turn drain using shared helper as safety net
The _handle_event() method looked for "chatItems" at the root level of
the WebSocket event dict, but the simplex-chat daemon nests the items
inside a "resp" envelope:
{"resp": {"type": "newChatItems", "chatItems": [...]}}
This meant that even when the daemon did push events (e.g. during
initial connection or after accepting a contact request), the adapter
always found an empty list and dropped every incoming message.
Wrap both the "newChatItem" singular and "newChatItems" batch paths
to extract the payload from event["resp"] before processing.
The simplex-chat daemon v6.5.1.1 does not push unsolicited events for incoming messages over the WebSocket. It only responds to commands. The adapter was designed to only listen for WebSocket events, which meant it never learned about new messages. Add _db_poll_loop() that reads the daemon SQLite database directly every 3 seconds, picks up chat_items with status rcv_new, converts them to MessageEvent, and hands them to handle_message(). Also wire up _poll_task lifecycle and chat_db_path config.
daimon-nous
Bot
added
type/bug
…eX send
send() method used @[{chat_id}] format (e.g. @[5]) to address messages.
The simplex-chat daemon requires contact display name (@display_name).
Both @5 (numeric ID) and @[5] (bracketed numeric) return chatCmdError.
Fix by storing display names via _contact_names dict (populated during DB
polling) and using @{display_name} in send(). Falls back to bare chat_id.
Also fixes _standalone_send() (out-of-process cron path) which had the
same @[{chat_id}] format issue.
Updates test_send_dm assertion for the new format and makes
test_standalone_send_missing_url tolerate daemon-online environments.
6065caa to
a8560ed
Compare
|
Interesting note on Bug 2: in my testing against simplex-chat (same v6.5.x range), the daemon does push newChatItems events asynchronously, but only after the WebSocket connection sends /_start first. Without that, the connection just sits idle. Adding await ws.send({"corrId": "...", "cmd": "/_start"}) immediately after connect made events flow normally on a single WebSocket, which would avoid the DB polling overhead. Possible the polling is more robust for edge cases, but the /_start approach worked end-to-end in my setup. |
|
@flyingeagles123 Thanks for testing this and sharing, really valuable insight! The To clarify the context behind Bug 2: in my testing against simplex-chat A couple of thoughts on combining both:
Would that align with what you observed? Happy to put together a follow-up PR that implements the dual approach if we can validate it end-to-end. |
cross-ref Adds a 2026-05-29 update: four consolidated fixes (/_start, resp.chatItems nesting, chatDir.groupMember sender, /_send syntax) on the new fix/simplex-event-dispatch branch (pushed to fork). Notes PR NousResearch#26433 already covers three of them, sender extraction is our novel contribution, and issue NousResearch#3 (silent drop) remains the open item to retest live now that /_start is in place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…emon Four independent bugs prevented end-to-end SimpleX messaging against a real simplex-chat daemon: 1. WS listener never sent /_start, so the daemon stored inbound messages but never pushed events to the subscriber. 2. newChatItems batch events nest `chatItems` under `resp`, not at the top level — every batched message was silently dropped. 3. Group sender was read from the legacy `chatItemMember` key; current simplex-chat reports it under `chatItem.chatDir.groupMember`, so sender_id fell back to the chat_id and failed allowlist matching. Falls back to chatItemMember for older payloads. 4. Outbound send used `@[id]`/`#[id]` bracket syntax the daemon reads as a literal contact name; switched to the `/_send @id text` / `/_send #id text` API form in both send() and _standalone_send(). Bugs 1, 2 and 4 mirror upstream PR NousResearch#26433 (issue NousResearch#30150). Bug 3 is not covered there; the chatDir.groupMember approach matches PRs NousResearch#4666/NousResearch#27978. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Independently reproduced all three of these bugs on a live deployment One fourth, related bug that this PR doesn't cover, in case it's useful to Minimal diff: - # Sender — for groups the message includes a chatItemMember sub-object
- member = chat_item.get("chatItemMember") or {}
+ # current simplex-chat reports the group member under chatDir.groupMember
+ chat_dir = chat_item.get("chatDir") or {}
+ member = chat_dir.get("groupMember") or chat_item.get("chatItemMember") or {}
if is_group and member:
+ member_profile = member.get("memberProfile") or {}
sender_id = str(
member.get("memberId")
+ or member.get("groupMemberId")
or member.get("id")
or chat_id
)
sender_name = (
member.get("displayName")
or member.get("localDisplayName")
+ or member_profile.get("displayName")
or sender_id
)Separately, a small heads-up on this PR's scope: the I've opened #35046 with the sender fix as a focused change + regression |
…ening Adds a 2026-05-29 part-2 update: sender fix upstreamed as issue NousResearch#35045 / PR NousResearch#35046; rationale for NOT opening a group-send PR (already covered by NousResearch#4666/NousResearch#27978 with a more robust json form); production branch hardened to the /_send <ref> json form (commit d31a043) to stop multi-line reply truncation; and a rebase note for the eventual NousResearch#26433 merge conflict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Quick follow-up with field results: I've now confirmed the first two fixes Restating the two gaps this PR doesn't close, for whoever triages:
Not blocking this PR — these are just the remaining pieces for full |
|
Thanks for this — you correctly identified two of the three bugs (the We merged the fix via #37045, using @maxcz79's #27120 as the base since it covered the same nesting + addressing bugs plus the idle-WebSocket reconnect churn, with a smaller, more surgical diff. One note for context: the field report on this issue (from a user running the daemon live) showed the daemon does push Both of you are credited in the merged PR. Appreciate you digging into the adapter and pinpointing the parsing bug. Closing this as superseded by #37045. |
6 participants
What does this PR do?
Fixes 3 bugs in the SimpleX Chat plugin (
plugins/platforms/simplex/adapter.py) that prevented bidirectional messaging:Bug 1 — Chat items looked up at wrong path
The
_handle_event()method checks forchatItemsat the root level of the WebSocket event dictionary, but the simplex-chat daemon nests them inside arespenvelope:{"resp": {"type": "newChatItems", "chatItems": [...]}}This meant that even when the daemon did push events (e.g. during initial connection or after accepting a contact request), the adapter always found an empty list and silently dropped every incoming message.
Bug 2 — Adapter assumed daemon pushes events
The adapter was designed to only listen for WebSocket events. Testing showed that simplex-chat v6.5.1.1 does not push unsolicited
newChatItemsevents for incoming messages. The WebSocket API is request-response only, with the sole exception of asubscriptionStatusevent sent on initial connection. As a result, the adapter never learned about new messages.Added a
_db_poll_loop()that reads the daemon's SQLite database directly every 3 seconds, picks upchat_itemswith statusrcv_new, converts them toMessageEvent, and hands them tohandle_message(). This avoids relying on the WebSocket push mechanism entirely.Bug 3 — Wrong send message format
The
send()method used@[{chat_id]to address messages (e.g.@[5]). The daemon requires the contact's display name — both@5(numeric ID) and@[5]returnchatCmdError. The same issue existed in_standalone_send()(used byhermes cronfor out-of-process delivery).Fixed by storing contact display names in a
_contact_namesdict (populated during the DB polling loop), using@{display_name}insend(), and switching_standalone_sendto@{chat_id}(no brackets) as a safe fallback for the cron path.Related Issue
No issue filed — found during hands-on testing of the adapter against a simplex-chat daemon v6.5.1.1.
Type of Change
Changes Made
plugins/platforms/simplex/adapter.py: UnwrapchatItemsfromrespenvelope in_handle_event()(bothnewChatItemsingular andnewChatItemsbatch paths)plugins/platforms/simplex/adapter.py: Add_db_poll_loop()method,_poll_tasklifecycle (create on connect, cancel on disconnect), andchat_db_pathconfigplugins/platforms/simplex/adapter.py: Fixsend()to use@{display_name}instead of@[{chat_id}]; store_contact_namesduring pollingplugins/platforms/simplex/adapter.py: Fix_standalone_send()to use@{chat_id}(no brackets)tests/gateway/test_simplex_plugin.py: Updatetest_send_dmassertion for new@{chat_id}formattests/gateway/test_simplex_plugin.py: Updatetest_standalone_send_missing_urlto tolerate both daemon-online and daemon-offline environmentsHow to Test
pytest tests/gateway/test_simplex_plugin.py -q— all 27 should passChecklist
Code
fix(scope):,test(scope):)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Before fix: Gateway log shows no inbound SimpleX messages. Polling loop not present.
After fix: Messages processed within seconds.