mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-31 19:16:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix(relay): prompts trust the run.py thread stamp — no self-anchor re-derivation (QA-5)

Browse source 
The threading mode (flat vs thread-per-message) is decided once, in run.py's
_resolve_progress_thread_id (reply_in_thread knob), and encoded in the
metadata stamp: flat => no thread_id, threaded => thread_id for the turn
(first turn: == message_id, the synthetic root IS the thread).

_strip_synthetic_dm_thread re-derived the mode with an unconditional
thread_id == message_id strip, exiling approval/clarify cards (and their
resolved-state swaps) to the DM root while progress bubbles honoured the
thread (2026-07-27 mixed-placement report). Trust the stamp instead; flat
mode is unaffected because flat metadata never carries an anchor.
This commit is contained in:
Victor Kyriazakos 2026-07-27 13:50:13 +00:00
parent 42e4f70eef
commit 95103db645
2 changed files with 77 additions and 55 deletions
Download patch file Download diff file Expand all files Collapse all files

28
gateway/relay/adapter.py
View file

@ -1301,17 +1301,23 @@ class RelayAdapter(BasePlatformAdapter):
thread_id = metadata.get("thread_id")
if not thread_id:
return metadata
# A real thread carries a thread_id distinct from the triggering message
# ts (run.py stamps that ts as metadata["message_id"] on Slack). Only the
# synthetic self-anchor (thread_id == that ts, or no distinguishing anchor
# present) is stripped; a genuine thread is honoured.
anchor = metadata.get("message_id")
if anchor is not None and str(thread_id) != str(anchor):
return metadata
cleaned = dict(metadata)
cleaned.pop("thread_id", None)
cleaned.pop("thread_ts", None)
return cleaned
# Trust the run.py stamp (QA-5). The threading MODE is decided in ONE
# place — run.py's _resolve_progress_thread_id, which reads
# platforms.slack.extra.reply_in_thread:
# * flat mode (reply_in_thread=false): the synthetic self-anchor is
# suppressed THERE, so prompt metadata arrives with NO thread_id and
# this helper is a no-op — the card posts flat at the DM root;
# * thread-per-message mode (default): metadata.thread_id is stamped
# for the whole turn, and on the FIRST turn it legitimately equals
# the triggering message's ts (the synthetic root IS the thread).
# The previous unconditional thread_id == message_id strip re-derived
# the mode here and got it wrong for thread-per-message: the approval
# card (and its resolved-state swap) was exiled to the DM root while
# progress bubbles honoured the thread (2026-07-27 mixed-placement
# screenshot). Mirror native SlackAdapter._resolve_thread_ts, which
# only performs the self-anchor strip when reply_in_thread=false — a
# state this lane never sees with an anchor present, per the above.
return metadata
async def _send_prompt(
self,

104
tests/gateway/relay/test_relay_slack_prompt_dm_root.py
View file

@ -1,21 +1,24 @@
"""Slack relay: interactive prompts (approval / clarify) must post FLAT at the DM root.
"""Slack relay: interactive prompts follow the turn's thread stamp (QA-5).
Reported symptom (live): an approval / clarify Block Kit card raised mid-turn in
a Slack DM was posted THREADED under the triggering message instead of at the DM
root ("the approval block was put in a thread and did not follow the setting").
The threading MODE (flat DM vs thread-per-message) is decided in exactly ONE
place: run.py's ``_resolve_progress_thread_id``, which reads
``platforms.slack.extra.reply_in_thread`` and encodes the verdict into the
outbound ``metadata`` stamp:
Root cause: a clarify/approval prompt is emitted in reply to the triggering
inbound event, so the metadata handed to ``_send_prompt`` carries that event's
thread context — run.py's ``_thread_metadata_for_source`` stamps
``metadata["thread_id"]`` (for a Slack DM, the triggering message's own ts, used
only as a session-keying fallback), plus ``metadata["message_id"]`` = that same
ts. Forwarding ``thread_id`` makes the connector's slackRestSender thread the
prompt card UNDER the user's message. Native Slack Hermes suppresses this
synthetic DM thread anchor (``SlackAdapter._resolve_thread_ts``); the relay lane
had no such disambiguation.
* flat mode -> the synthetic self-anchor is suppressed in run.py, so prompt
metadata arrives with NO ``thread_id`` and the card posts at the DM root;
* thread-per-message (default) -> ``metadata.thread_id`` is stamped for the
whole turn; on the FIRST turn it legitimately equals the triggering
message's ts (the synthetic root IS the thread).
These are behaviour-contract tests: they assert how the outbound ``prompt`` frame
relates to the chat type + inherited thread metadata (the invariant the connector
The prompt lane must TRUST that stamp, like ``_resolve_reply_to_for_send``
does. Re-deriving the mode here (the old unconditional
``thread_id == message_id`` strip) exiled the approval card and its
resolved-state swap to the DM root while progress bubbles honoured the thread
(the 2026-07-27 mixed-placement report).
These are behaviour-contract tests: they assert how the outbound ``prompt``
frame relates to the inherited thread metadata (the invariant the connector
depends on), not a snapshot. They drive the REAL ``RelayAdapter`` +
``StubConnector`` end to end.
"""
@ -83,15 +86,39 @@ def _last_prompt(stub) -> dict:
# ---------------------------------------------------------------------------
# DM-root: the synthetic self-anchor is stripped
# Flat mode: run.py stamps NO thread_id -> the card posts at the DM root.
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_exec_approval_posts_flat_at_dm_root():
"""A Slack DM approval prompt must NOT inherit the triggering message's
synthetic thread_id — it posts flat at the DM root, matching native."""
async def test_exec_approval_flat_mode_posts_at_dm_root():
"""Flat-DM turn (reply_in_thread=false): run.py suppressed the synthetic
anchor upstream, so prompt metadata has no thread_id and none appears on
the wire — the card posts at the DM root."""
adapter, stub = _wire("D1", "dm", scope_id="T1")
md = {"message_id": "1700000000.000100", "scope_id": "T1"}
result = await adapter.send_exec_approval(
"D1", "rm -rf /tmp/x", "sess:1", description="deletes files", metadata=md
)
assert result.success is True
frame = _last_prompt(stub)
meta = frame["metadata"] or {}
assert "thread_id" not in meta
assert "thread_ts" not in meta
# reply_to on the outbound action stays unset — a root-level post.
assert frame["reply_to"] is None
# Tenant scope is preserved untouched (egress routing must not break).
assert meta.get("scope_id") == "T1"
# ---------------------------------------------------------------------------
# Thread-per-message mode: the first-turn self-anchor (thread_id == message_id)
# IS the thread root — the prompt must stay in the thread (QA-5 regression).
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_exec_approval_first_turn_self_anchor_stays_in_thread():
"""Thread-per-message first turn: run.py stamps thread_id = the triggering
message's own ts. The approval card must post INTO that thread — stripping
it exiled the card to the home channel (2026-07-27 report)."""
adapter, stub = _wire("D1", "dm", scope_id="T1")
# run.py hands the prompt the triggering message's thread context: for a DM
# with no real thread, thread_id == message_id (the synthetic self-anchor).
md = {
"thread_id": "1700000000.000100",
"message_id": "1700000000.000100",
@ -103,22 +130,14 @@ async def test_exec_approval_posts_flat_at_dm_root():
assert result.success is True
frame = _last_prompt(stub)
meta = frame["metadata"] or {}
# The inherited synthetic thread anchor is dropped so it posts at the DM root.
assert "thread_id" not in meta, (
"approval prompt must NOT inherit the triggering message thread_id"
assert meta.get("thread_id") == "1700000000.000100", (
"first-turn self-anchor is the thread root; the prompt must honour it"
)
assert "thread_ts" not in meta
# reply_to on the outbound action stays unset — a root-level post.
assert frame["reply_to"] is None
# Tenant scope is preserved untouched (egress routing must not break).
assert meta.get("scope_id") == "T1"
# The caller's original metadata dict was not mutated in place.
assert md.get("thread_id") == "1700000000.000100"
@pytest.mark.asyncio
async def test_clarify_posts_flat_at_dm_root():
"""A Slack DM clarify prompt (with choices) also posts flat at the DM root."""
async def test_clarify_first_turn_self_anchor_stays_in_thread():
adapter, stub = _wire("D1", "dm", scope_id="T1")
md = {
"thread_id": "1700000000.000200",
@ -131,22 +150,21 @@ async def test_clarify_posts_flat_at_dm_root():
assert result.success is True
frame = _last_prompt(stub)
meta = frame["metadata"] or {}
assert "thread_id" not in meta
assert "thread_ts" not in meta
assert frame["reply_to"] is None
assert meta.get("thread_id") == "1700000000.000200"
assert meta.get("scope_id") == "T1"
@pytest.mark.asyncio
async def test_slash_confirm_posts_flat_at_dm_root():
"""The DM-root rule covers every prompt surface (single _send_prompt choke)."""
async def test_slash_confirm_first_turn_self_anchor_stays_in_thread():
"""The stamp-trusting rule covers every prompt surface (single
_send_prompt choke point)."""
adapter, stub = _wire("D1", "dm")
md = {"thread_id": "1700000000.000300", "message_id": "1700000000.000300"}
await adapter.send_slash_confirm(
"D1", "Reload MCP", "invalidates cache", "s", "cf-1", metadata=md
)
frame = _last_prompt(stub)
assert "thread_id" not in (frame["metadata"] or {})
assert (frame["metadata"] or {}).get("thread_id") == "1700000000.000300"
# ---------------------------------------------------------------------------
@ -155,8 +173,7 @@ async def test_slash_confirm_posts_flat_at_dm_root():
@pytest.mark.asyncio
async def test_exec_approval_in_real_thread_keeps_thread_id():
"""A DM prompt raised inside a REAL thread (thread_id distinct from the
triggering message ts) must STAY in that thread — only the synthetic
self-anchor is stripped."""
triggering message ts) stays in that thread."""
adapter, stub = _wire("D1", "dm", scope_id="T1")
md = {
"thread_id": "1699000000.999000",
@ -170,8 +187,7 @@ async def test_exec_approval_in_real_thread_keeps_thread_id():
@pytest.mark.asyncio
async def test_channel_approval_keeps_thread_id():
"""A Slack CHANNEL prompt keeps its thread_id (autoThread / real thread);
the DM-only guard must not touch a non-DM chat."""
"""A Slack CHANNEL prompt keeps its thread_id (autoThread / real thread)."""
adapter, stub = _wire("C1", "channel", scope_id="T1")
md = {
"thread_id": "1700000000.000400",
@ -185,8 +201,8 @@ async def test_channel_approval_keeps_thread_id():
@pytest.mark.asyncio
async def test_non_slack_dm_approval_keeps_thread_id():
"""The disambiguation is Slack-scoped: a non-Slack relay DM keeps thread_id
(its connector owns its own threading semantics)."""
"""A non-Slack relay DM keeps thread_id (its connector owns its own
threading semantics)."""
adapter, stub = _wire("dc1", "dm", platform=Platform.DISCORD)
md = {"thread_id": "9000", "message_id": "9000"}
await adapter.send_exec_approval("dc1", "cmd", "s", metadata=md)