mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(webhook): honor [SILENT] when the agent explains its own silence
A webhook route that answered `[SILENT]` still delivered, whenever the model
added a sentence saying why it was staying quiet:
[SILENT]
The new inbound was the same email quoted back a second time, on a ticket
we already answered. Nothing new to reply to, so I closed it.
Webhook subscription prompts tell the agent to answer `[SILENT]` on a tick that
produced no story — a duplicate inbound, a stand-down because a sibling lane
already replied, a routine close. Nobody is waiting on the other end of a
webhook, so a "nothing happened" message has no reader.
Delivery went through the live gateway's `is_intentional_silence_response`,
which requires the response to be EXACTLY a marker. That rule is right for an
interactive chat: swallowing a real answer because it opens with a marker is
much worse than showing a stray marker. It is the wrong trade for an autonomous
lane, where a leaked non-story is a pointless notification on every tick and
models reliably append the explanation that flips the check back to "deliver".
Cron already resolved this the other way — `cron/scheduler.py` treats a marker
on its own first or last line as silence — so the two autonomous lanes
disagreed while the interactive path was fine.
Suppress in `WebhookAdapter.send`, before the deliver-type switch, so every
route (log, github_comment, cross-platform) behaves the same. Reuses cron's
`_is_cron_silence_response` rather than restating the rule, so the two lanes
cannot drift; prose that merely mentions a marker mid-sentence still delivers.
The interactive gateway path is untouched.
Tests: six cases in tests/gateway/test_webhook_adapter.py — bare marker,
marker + trailing prose (the reported shape), marker on the last line, a real
report, a report quoting a marker mid-sentence, and a `log` route. Verified
red-first: with the suppression removed the three silence cases fail
("Expected send to not have been awaited") while the three delivery cases still
pass, so the tests assert the fix rather than the framework.
This commit is contained in:
parent
bd6437d605
commit
55d3272286
2 changed files with 151 additions and 0 deletions
|
|
@ -66,6 +66,40 @@ from gateway.platforms.webhook_filters import (
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _is_webhook_silence_response(content: Any) -> bool:
|
||||
"""Whether an agent response means "deliberately say nothing".
|
||||
|
||||
Webhook routes are autonomous background lanes: a subscription prompt tells
|
||||
the agent to answer with ``[SILENT]`` when a tick produced nothing worth a
|
||||
human's attention (a duplicate inbound, a stand-down because a sibling lane
|
||||
already replied, a routine close). Nobody is waiting on the other end, so
|
||||
there is no reader for whom a "nothing happened" message is useful.
|
||||
|
||||
The reason this is cron's looser rule rather than the live gateway's is what
|
||||
the two lanes optimise for. In an interactive chat, swallowing a real answer
|
||||
because it happens to open with a marker is much worse than showing a stray
|
||||
marker, so ``is_intentional_silence_response`` demands the response be
|
||||
EXACTLY a marker. A webhook run has the opposite payoff: the cost of a
|
||||
leaked non-story is a pointless notification on every tick, and models
|
||||
reliably add a sentence explaining why they stayed quiet — which under the
|
||||
strict rule flips the whole thing back to "deliver". That is not a
|
||||
hypothetical: it is why a Helper support lane kept messaging its owner to
|
||||
report that it had nothing to report.
|
||||
|
||||
So reuse cron's matcher, which already treats a marker on its own first or
|
||||
last line as silence while still delivering prose that merely mentions one
|
||||
mid-sentence. Sharing the function keeps the two autonomous lanes from
|
||||
drifting apart, and keeps the interactive path untouched.
|
||||
"""
|
||||
if not isinstance(content, str):
|
||||
return False
|
||||
try:
|
||||
from cron.scheduler import _is_cron_silence_response
|
||||
except Exception: # pragma: no cover - cron package always ships with the gateway
|
||||
return False
|
||||
return _is_cron_silence_response(content)
|
||||
|
||||
# Sentinel returned by _resolve_request_profile when a /p/<profile>/ prefix
|
||||
# names a profile this gateway does not serve (→ 404). Distinct from None
|
||||
# (no prefix / multiplexing off → handle as the default profile).
|
||||
|
|
@ -336,6 +370,12 @@ class WebhookAdapter(BasePlatformAdapter):
|
|||
do not consume the entry and silently downgrade the final response
|
||||
to the ``log`` deliver type. TTL cleanup happens on POST.
|
||||
"""
|
||||
if _is_webhook_silence_response(content):
|
||||
logger.info(
|
||||
"[webhook] Response for %s is a silence marker — not delivering", chat_id
|
||||
)
|
||||
return SendResult(success=True)
|
||||
|
||||
delivery = self._delivery_info.get(chat_id, {})
|
||||
deliver_type = delivery.get("deliver", "log")
|
||||
|
||||
|
|
|
|||
|
|
@ -1296,6 +1296,117 @@ class TestSessionIsolation:
|
|||
assert len(ids) == 2, "Each delivery must have a unique session chat_id"
|
||||
|
||||
|
||||
# ===================================================================
|
||||
# Silence-marker suppression
|
||||
# ===================================================================
|
||||
|
||||
|
||||
class TestWebhookSilenceSuppression:
|
||||
"""A webhook route that answers ``[SILENT]`` must deliver nothing.
|
||||
|
||||
Webhook routes are autonomous lanes with nobody waiting on the other end,
|
||||
so a subscription prompt tells the agent to reply ``[SILENT]`` on a tick
|
||||
that produced no story. Models routinely append a sentence saying WHY they
|
||||
stayed quiet, and the live gateway's exact-whole-response rule then treats
|
||||
that as a real report — which is how a Helper support lane ended up
|
||||
repeatedly messaging its owner to say it had nothing to say.
|
||||
"""
|
||||
|
||||
def _adapter_with_mock_target(self):
|
||||
adapter = _make_adapter()
|
||||
mock_target = AsyncMock()
|
||||
mock_target.send = AsyncMock(return_value=SendResult(success=True))
|
||||
mock_runner = MagicMock()
|
||||
mock_runner.adapters = {Platform("telegram"): mock_target}
|
||||
mock_runner.config.get_home_channel.return_value = None
|
||||
adapter.gateway_runner = mock_runner
|
||||
|
||||
chat_id = "webhook:helper-events:d-1"
|
||||
adapter._delivery_info[chat_id] = {
|
||||
"deliver": "telegram",
|
||||
"deliver_extra": {"chat_id": "-100123"},
|
||||
}
|
||||
adapter._delivery_info_created[chat_id] = time.time()
|
||||
return adapter, mock_target, chat_id
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bare_marker_is_not_delivered(self):
|
||||
adapter, target, chat_id = self._adapter_with_mock_target()
|
||||
|
||||
result = await adapter.send(chat_id, "[SILENT]")
|
||||
|
||||
assert result.success is True
|
||||
target.send.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_marker_followed_by_prose_is_not_delivered(self):
|
||||
"""The regression this suppression exists for.
|
||||
|
||||
The agent explains its own silence on the lines after the marker. The
|
||||
strict interactive rule reads that as substantive prose and delivers the
|
||||
whole thing, marker included.
|
||||
"""
|
||||
adapter, target, chat_id = self._adapter_with_mock_target()
|
||||
|
||||
result = await adapter.send(
|
||||
chat_id,
|
||||
"[SILENT]\n\nThe new inbound was the same email quoted back a second "
|
||||
"time, on a ticket we already answered. Nothing new to reply to, so I "
|
||||
"closed it; it reopens by itself if they write back.",
|
||||
)
|
||||
|
||||
assert result.success is True
|
||||
target.send.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_marker_on_the_last_line_is_not_delivered(self):
|
||||
adapter, target, chat_id = self._adapter_with_mock_target()
|
||||
|
||||
result = await adapter.send(chat_id, "Nothing to report this tick.\n\n[SILENT]")
|
||||
|
||||
assert result.success is True
|
||||
target.send.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_real_report_is_still_delivered(self):
|
||||
"""Suppression must not swallow an actual story."""
|
||||
adapter, target, chat_id = self._adapter_with_mock_target()
|
||||
|
||||
result = await adapter.send(
|
||||
chat_id,
|
||||
"Refunded $240 to the buyer and replied; the seller had already agreed.",
|
||||
)
|
||||
|
||||
assert result.success is True
|
||||
target.send.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_report_mentioning_the_marker_mid_sentence_is_delivered(self):
|
||||
"""A report that merely quotes a marker is not a silence request."""
|
||||
adapter, target, chat_id = self._adapter_with_mock_target()
|
||||
|
||||
result = await adapter.send(
|
||||
chat_id,
|
||||
"I considered staying [SILENT] but this one moved money, so: refunded "
|
||||
"$240 and replied to the buyer.",
|
||||
)
|
||||
|
||||
assert result.success is True
|
||||
target.send.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_suppression_precedes_log_delivery(self):
|
||||
"""A `log` route also suppresses, so the two lanes behave the same."""
|
||||
adapter = _make_adapter()
|
||||
chat_id = "webhook:helper-events:d-log"
|
||||
adapter._delivery_info[chat_id] = {"deliver": "log", "deliver_extra": {}}
|
||||
adapter._delivery_info_created[chat_id] = time.time()
|
||||
|
||||
result = await adapter.send(chat_id, "[SILENT]\n\nnothing happened")
|
||||
|
||||
assert result.success is True
|
||||
|
||||
|
||||
# ===================================================================
|
||||
# Delivery info cleanup
|
||||
# ===================================================================
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue