Skip to content

Hermes: one bridge per gateway, shared by its chats - #154

Open
zaydiscold wants to merge 1 commit into
reason-machines:mainfrom
zaydiscold:feat/hermes-shared-bridge
Open

zaydiscold wants to merge 1 commit into
reason-machines:mainfrom
zaydiscold:feat/hermes-shared-bridge

Conversation

@zaydiscold

Copy link
Copy Markdown
Contributor

Follow-up to #119, answering the question there: one bridge per gateway works, so the plugin now runs one per Hermes process and shares it across its conversations.

Why

I run two gateways. One serves a single long Telegram chat; the other has several desktop tabs open plus delegated subagents, and today it started dozens of conversations, many within the same minute. Each one started its own reason-network bridge and daemon connection, and a conversation that was never finalized kept both. For the single-chat gateway nothing changes: it's one bridge either way.

What changes

  • Daemon. agent::shares_connection() (the multiplexed MCP agents, plus hermes) replaces multiplexed() in the two places that assume one conversation per connection: register no longer ends the other Hermes sessions on that connection (moved_on), and the client's reconnect memory keeps all of them, so a daemon restart registers every open chat again. multiplexed() itself is unchanged, so Hermes's MCP-server path behaves as before.
  • end_session. A new request ends one conversation on a shared connection. Only the connection that registered it can end it, and the client forgets it so a reconnect doesn't bring it back. The bridge drops that conversation's received set at the same time, so a long-running gateway doesn't keep every old chat's ids.
  • Frames. With every chat on one bridge, a bad frame shouldn't end the process. An oversized or non-UTF-8 frame now fails that call (id 0) and the bridge keeps reading. The plugin refuses a frame over 1 MiB before writing it, so its own calls fail at once instead of timing out.
  • Plugin. One Bridge on Network, with deliveries routed by key. finalize sends end_session instead of closing a process. If the bridge stops, the next turn or tool call starts a new one and that conversation registers again as itself. The per-conversation locks are as before, and nothing holds _lock across I/O.

Tradeoffs

  • Isolation. If the shared bridge dies, every chat in that process loses its connection at once rather than one. Nothing is lost: mail stays unread and each chat registers again on its next turn or tool call (tested).
  • Identity. Within one Hermes process the bridge knows all of that process's keys, so per-conversation identity is enforced by the plugin, which only ever passes a conversation its own key. Across processes nothing changes: another Hermes's conversations are refused, and the existing test now checks exactly that.

Tests

  • Rust. A unit test for three chats on one connection, where another connection can't end one and dropping the connection ends the rest. The reconnect-memory test also covers Hermes. cargo test 255 + 2, clippy, fmt.
  • hermes.py. 25/25, against both hermes-agent#143310 as installed and #143310 rebased onto current main. New: conversations share one adapter; a killed adapter is replaced and both conversations register again; an ended conversation stays gone after reason-network stop; mail held for a conversation reaches it when it comes back; an oversized frame fails only its call. Updated: the three tests that asserted a closed process now assert the conversation left the directory, and the stand-in engine answers concurrently, as the real bridge does.
  • Other e2e. hooks.py 74/74, reconnect.py 41/41, names.py 32/32. mixed_versions.py passes 21/23, and the same 2 fail on main when the "old" binary is 0.12.13, which can already read rn1 links.
  • Mutation check. I broke each new behavior on purpose. 9 of 10 were caught. The miss was routing deliveries by a key map filled in before register instead of by stored sessions: the daemon's push arrived after the plugin stored the session either way, so I kept the map as a guard and don't claim a race fix for it.

I haven't run it on native Windows yet; #119's Windows checks passed, and nothing here is platform-specific beyond the existing pipe transport.

One thing I hit while testing on my own machines: a plugin from 0.12.13 with an engine from 0.12.12 or earlier (no bridge subcommand) logs only "Reason adapter connection closed" on every turn. Running reason update and then reason network setup fixed it on both. A clearer message for that case could be a small separate PR if you'd like one.

A Hermes gateway serving many chats ran one `reason-network bridge` per
conversation: a process and a daemon connection each. They now share one.

- The daemon lets a Hermes connection carry many sessions: registering one
  no longer ends the others (`agent::shares_connection`), and a reconnect
  registers every open one again.
- `end_session` ends one conversation on a shared connection. Only the
  connection that registered it can end it, and a reconnect leaves it out.
- An oversized or non-UTF-8 frame fails that call (id 0) instead of ending
  the bridge, since the bridge now carries every chat. The plugin refuses
  such a frame before sending it.
- The plugin keeps one bridge, routes deliveries by key, ends a closed
  conversation with `end_session`, and registers each conversation again
  on a new bridge if the old one stops.

Tests: three chats on one connection with only the registering connection
able to end one (daemon), reconnect memory keeps every Hermes chat
(client), and e2e for the shared adapter, a restarted adapter, an ended
conversation staying gone after a daemon restart, held mail arriving when
a conversation comes back, and an oversized frame failing only its call.
@larstalian

Copy link
Copy Markdown
Member

Thanks, and for rebasing hermes-agent#143310. Reviewing now, including the full suite and the Hermes e2e against #143310. I will report back here.

@larstalian

Copy link
Copy Markdown
Member

Sorry for the churn: main moved quite a bit in the last two days (v0.12.14 to v0.12.17, with changes in mail.rs, store.rs, proto.rs and the daemon), and GitHub now shows conflicts here. Could you rebase onto current main when you get a chance? We'll review the rebased head right away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants