Skip to content

fix(relay): keep interaction and message identifiers distinct - #130309

Open
dskwe wants to merge 3 commits into
NousResearch:mainfrom
dskwe:fix/relay-message-identities-130306
Open

dskwe wants to merge 3 commits into
NousResearch:mainfrom
dskwe:fix/relay-message-identities-130306

Conversation

@dskwe

@dskwe dskwe commented Oct 1, 2026

Copy link
Copy Markdown

What does this PR do

Keeps Discord interaction identifiers separate from actual Discord message identifiers in the relay lane. Component interactions use the attached message as the event and source message target, while application commands do not invent a message ID. The interaction ID remains available in event metadata for acknowledgement and tracing. Relay wire decoding also normalizes a root-only message ID into the source.

Related Issue

Fixes #130306

Type of Change

  • Bug fix

Changes Made

  • gateway/relay/adapter.py: map attached Discord component messages to message targets and preserve interaction IDs separately.
  • gateway/relay/ws_transport.py: normalize root and nested relay message IDs.
  • tests/gateway/relay/test_relay_interactive.py: cover component and application-command identity behavior.

How to Test

  • /Users/liyuan/github/ai/hermes-agent/venv/bin/python3 -m pytest tests/gateway/relay/test_relay_interactive.py tests/gateway/relay/test_ws_transport.py -q
  • Result: 28 passed, 1 warning in 10.67s

Checklist

  • The change is limited to the reported behavior.
  • Tests cover the changed behavior.
  • No credentials or generated files are included.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Oct 1, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Reviewed at head 5af4e398. The identity split is right and I would not ask you to roll it back. I reproduced all three of #130306's cases on base first: base set source.message_id = payload["id"] (the interaction id) on every shape, so reaction acks and HERMES_SESSION_MESSAGE_ID read an interaction id as a message target. The head fixes all three — slash/modal → None, component → the real message.id, root-only frame → source and event agree.

To finish: nulling source.message_id on slash re-renders the pinned prompt

source.message_id's presence is an input to _ephemeral_change_key (gateway/run_agent_cache.py) and decides whether _discord_platform_notes (gateway/session.py) emits the - Triggering message: provided per-turn… line. Against a relayed text message in the same chat, on base slash, component and modal all land on one pin key with that line present; on head component stays on the text key but slash and modal drop to another, with it absent.

Text → slash → text therefore re-keys the pin twice: turn 2 re-renders the system prompt, turn 3 re-renders it back. AGENTS.md makes prompt caching the repo's first invariant, and rewriting already-sent system bytes mid-conversation is what it warns against. This is the trap #130306 flagged — a split "cannot simply blank source.message_id for slash commands (the regression teknium1 measured on #129032)."

So you don't over-correct: it is not relay-only. Native Discord already behaves this way — _build_slash_event (plugins/platforms/discord/adapter.py:4724) never passes message_id to build_source. The head aligns relay with the native lane rather than inventing a new bad state, so keep the split and finish it: render the guidance from a lane-independent presence bit rather than source.message_id's emptiness, and switch that component of _ephemeral_change_key to match.

Minor: the ws_transport change has no test

Reverting that line — undoing this file's whole behaviour change — leaves all 37 tests across the ws-transport, passthrough, dedupe and wire-identity relay tests green, so the root-only-frame normalisation listed as one of your three changes is unasserted. The issue's regressions list asks for a source.message_id is not None assertion on that frame.

Evidence: rb/130309_r1.md.

@dskwe

dskwe commented Oct 4, 2026

Copy link
Copy Markdown
Author

@Enough1122 Addressed in separate fix/test commits 53cd7a165dfb3698ecb1ee9001fd4d87cfc4b0d5 and e2896f8ca72e77bed5f2445ae1866a1f94bc5026.

The interaction/message identity split is unchanged: slash and unattached modal turns have no message target, components use the attached message ID, and the interaction ID remains in metadata only.

_discord_platform_notes now renders triggering-message guidance from the existing Discord-tools capability gate, independent of the current source's message-ID presence. Its wording explicitly permits an absent per-turn target and forbids substituting an interaction ID. _ephemeral_change_key retains the same capability gate but no longer includes message-ID presence. This applies equally to native and relayed Discord, without adding a synthetic message ID or a relay-only flag.

Behavioral coverage:

  • Real relay text/interaction converters exercise text → slash/component/modal → text with Discord tools enabled. They assert identical rendered guidance and keys, reuse of the same pinned string object, and exactly one renderer call. The session-env and incoming-user-message consumers receive the actual message target or no target, never the interaction ID or the previous turn's target.
  • The native _build_slash_event path also exercises text → slash → text prompt continuity. Additional tests cover both tool-gate states and verify that an actual capability change still changes the guidance and key.
  • The real in-process WebSocket transport now receives both root-only and root-plus-nested message-ID frames; both assert source.message_id is not None and equality with event.message_id.

Evidence: before the prompt fix, the new relay slash/modal and native slash continuity cases failed (3 failed, 3 passed). Temporarily reverting only the root-ID normalization made the root-only socket case fail at the explicit non-null assertion while the nested case passed (1 failed, 1 passed); that mutation was restored before the final runs.

Using the shared venv's python3 -m pytest, final results:

  • tests/gateway/relay/test_relay_interactive.py tests/gateway/relay/test_ws_transport.py tests/gateway/test_discord_slash_commands.py tests/gateway/test_prompt_tail_freeze.py -q: 60 passed, 1 warning in 14.36s.
  • tests/gateway/relay tests/gateway/test_session.py tests/gateway/test_session_env.py tests/gateway/test_internal_event_pin_wiring.py -q: 473 passed, 6 warnings in 54.35s.

Warnings are dependency/Python deprecations; both final runs have no failures. git diff --check is clean. No full-repository test run is claimed.

This branch has not been deployed

No deployments
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 platform/discord Discord bot adapter 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.

[Bug]: the relay lane mixes up interaction ids and message ids (source vs event message_id)

3 participants