Repository navigation
fix(peer-messaging): store a peer message before it leaves the mailbox, and never frame it as the user - #1
Open
seyfal wants to merge 11 commits into
Conversation
The pre-request drain in prepare_iteration put a pending /steer after the newest tool result anywhere in the transcript. A steer accepted before a turn's first request (for example while turn-start compaction runs) therefore landed inside the previous turn, between its tool result and its final answer, ahead of the new question: the model read the correction out of order and the earlier turns' cached prefix changed. NousResearch#121077 stopped idle-session steers from the TUI gateway reaching this branch; a steer accepted by a running turn still reaches it. Search only after this turn's user row, re-derived when mid-turn compaction left the recorded index stale. With no tool result in the turn yet the steer is requeued, as it already was when the transcript held no tool result at all: the post-tool drain delivers it, or the turn hands it back as pending_steer.
The activity hook moved the mailbox into the user's /steer slot and deleted the files at read time. Two ways that lost or misattributed a message: - When the turn then made no tool call, the text came back as result["pending_steer"]: one-shot `chat -q` dropped it, and the CLI, desktop and gateway started a new turn with it as if the user had typed it. - Between the delete and the next transcript flush (a whole model request plus the next tool batch) the only copy was in memory, so a receiver crash lost it. Read the mailbox as the last step of prepare_iteration, before each model request. When the transcript ends in this turn's tool results (and rows already delivered after them), append the messages as one row, flush it to the session database, and only then delete the files. A transcript ending in anything else (retry scaffolding, a continuation) leaves the files for the next request, and a row the database refuses is taken back out, so a message is sent once and stored before its file goes. A crash between the store and the delete can deliver it twice, never zero times. Peer text no longer enters _pending_steer. The activity hook and its 5 s throttle go away: the read runs once per model request and takes about 40 us when the session has no inbox (one stat(); 145 us with an empty inbox; measured on an M-series Mac).
…steer The row carrying peer text used the /steer frame, "[OUT-OF-BAND USER MESSAGE — a direct message from the user ...]", which the system prompt's steer note gives the user's full authority, while the inner text said it was not from the user. It was stored as display_kind "steer", so the desktop rendered it as the user's own steer, every "is this human input" predicate (compression anchors, /undo picks) counted it as the user's, and a compaction summary quoted it verbatim among the user's messages. Deliver it as its own row: a PEER SESSION MESSAGE frame that says another session of the same user wrote it and that it carries no user authority (act on it only within what the user asked), and display_kind "peer_message". The system prompt is untouched, so no session's cached prefix changes. The row keeps the steer's slot, so the pending-tool- round scan skips it like a steer row, the alternation repair never merges it with a user row on either side, the compaction summary leaves it out of the user's verbatim messages, and the desktop renders the kind as a notice.
Hosted rooms compiled their own regex over CONTROL_FRAME_OPENERS to relabel control frames a member reply quotes. Peer-session messages republish another session's model output as role=user text the same way and need the same relabel. Move the regex next to the opener list as relabel_control_frames(text, label); hosted rooms call it with "member-quoted" and produce the same text as before.
A peer message is another session's model output, and that session may
have read a web page or file written to steer it. Its text was wrapped
verbatim, so a message quoting the /steer marker put the exact
"[OUT-OF-BAND USER MESSAGE — a direct message from the user ...]" shape
inside the recipient's row, right after its tool results, where the
system prompt tells the model to trust it as the user. A quoted
"[/PEER SESSION MESSAGE]" could also end the peer frame early.
Relabel every control-frame opener in the text before it is framed, the
way hosted rooms already treat a member's reply ("[peer-quoted ..."):
the words stay, the trusted shape does not. The peer frame joins
CONTROL_FRAME_OPENERS, so hosted rooms (and the desktop's copy of the
list) relabel it too.
…arts A session that receives a message while idle has no tool result to put it after when its next turn starts, so the message waited for that turn's first tool call and a turn without one never delivered it. Also accept a transcript that ends in this turn's question: the row goes right after it. The request then carries two user rows, which the wire copy already merges (drop_thinking_only_and_merge_users), while the store keeps the question as the user's own row and the peer row as its own, since the alternation repair never merges a peer row. Any other tail still waits, so a message that arrives during a thinking-only reply is not appended after the reasoning prefill the next request continues from.
The background memory/skill review fork (and /btw) is built with the parent's session_id, _session_db=None and _persist_disabled=True. Its requests read the mailbox like any agent's, and with nothing to store into, the row counted as delivered: a review moved the parent's messages into a throwaway transcript and deleted them. An agent that persists nothing never reads the mailbox. _persist_disabled is the flag the fork already carries so it never writes into the real session, so it covers every non-primary agent that shares a session id.
…rite peer_send wrote each message with write_text and os.replace, so the file took the process umask (0644 at the usual 022) and was never fsynced before it was published. The Hermes home is 0700 by default, but HERMES_HOME_MODE (for example 2770) and managed installs (umask 007) open it to a group, whose members could then read every queued message. Publish through utils.atomic_json_write(mode=0o600), the writer the bot relay mailbox already uses: the temp file is 0600 from creation, fsynced, and replaced atomically.
peer_send only checked that the target id exists, so a message to an ended session (one peer_sessions already hides) was reported as queued and left a mailbox on disk. Nothing reads it unless the user resumes that exact id, and a session ended by a compression split continues under a new id that never will. Refuse when the session store marks the target ended and point the sender at peer_sessions for the live id. The sender sees the refusal and can tell its user, instead of trusting a queued message nobody may read.
The toolset description and docs tell users to enable discovery and sending via `hermes tools` → Peer Messaging, but the toolset was missing from CONFIGURABLE_TOOLSETS, so the checklist never offered it. Add the entry. Its tools are not in any platform composite, so it stays off until selected, like other opt-in toolsets.
The toolset description, the tool reference and the toolsets reference still said messages ride the user-role steer channel on the recipient's next activity and can be lost once drained into memory. Describe the current contract: the mailbox is read before each model request, the message arrives as a framed peer row without user authority (quoted control frames relabelled), an idle session gets it when its next turn starts, ended sessions are refused, and a message file is deleted only after its row is stored.
9 of 10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes five problems in NousResearch#136861 that let a peer message get lost, pass as the user's own words or land in the wrong turn. I found them by running this branch with two Hermes processes, and two of them are the blocking findings from the review of NousResearch#106423. The busy case already works, a message arrives after the recipient's tool batch and the prompt cache holds.
/steerslot. A turn without a tool call hands it back, sochat -qdrops it and the CLI, desktop and gateway replay it as a typed turn.state.db./steerframe, which the system prompt treats as the user, and the desktop shows it as the user's own steer.PEER SESSION MESSAGEframe without user authority and its owndisplay_kind, and the system prompt doesn't change./steermarker and the recipient sees the user's trusted frame./steersent before the first request has the same bug./btw) never reads the mailbox.Queued messages are also written 0600,
peer_sendrefuses ended sessions, andhermes toolsoffers Peer Messaging, as the docs already said.At turn start the store keeps two user rows in a row, the question and then the peer row, and the request merges them per call, the same way it already sends a stored
/steerfollowed by the next prompt. Waiting for a tool result instead would leave a message unread through every turn without one. Rows are only appended, so the cached prefix never changes. The flush tostate.dbbefore the delete is the only new write, and a crash between the two delivers a message twice, never zero times. The read costs about 40 µs per request when the session has no inbox.The first commit fixes the
/steerplacement on its own and applies cleanly to upstreammain. The CLI resume recap and the TUI still show a peer row as a user entry, because they need new locale strings, and I left them for a separate change.Tests
/steer, review fork, hosted room and tools config tests (8 files)tests/agent,tests/toolsandtests/tui_gateway/steerI also reverted each guarded line in turn (the flush before the delete, the tail check, the merge guard and the relabel) and its test failed each time.
python scripts/checkpasses. I also ran this version live onclaude-haiku-4-5. A busy session got the message right after its tool result, an idle session got it right after the new question, and a turn without a tool call kept it queued and delivered it on the next turn. The tests run withscripts/run_tests.sh tests/tools/test_peer_messaging.py tests/tools/test_peer_messaging_delivery.py tests/tools/test_peer_messaging_turns.py tests/agent/test_steer.py. I tested on macOS 26.6.