fix(gateway): a bare silence marker on a turn not addressed to the bot stays silent
Since5ea8fb2b78(#111624, for #110952) the gateway rejects a bare silence marker on any human turn and delivers "The model returned only a silence marker for a message that needed a reply" instead. That protects a human who asked this bot something and got nothing back. It also fires on every human message the adapter admitted without the bot being addressed at all: a free-response channel, a thread follow-up under `thread_require_mention: false`, or a message @-mentioning another person or bot with `ignore_other_user_mentions: false`. A bot whose SOUL declines peer-addressed turns with a deliberate marker now posts that notice on every such message. A fleet running several bots in shared Slack threads reported it as spam on v2026.9.21. #37940 established that intentional silence must not be re-inflated. Both contracts hold once the turn knows whether a reply was expected. `MessageEvent.reply_expected` (True, False, None) is set by the adapter where the message is admitted. Slack (`slack_reply_expected`): a 1:1 DM, an @mention of this bot or a command is True, anything else it admits is False. Other adapters leave None, which keeps today's behaviour, so nothing changes for them until they are ported. `response_filters.silence_allowed` holds the one rule (machinery turn, or reply not expected) and both call sites use it: the live turn in `run_turn._hmwa_shape_agent_response` and the crash-recovery redelivery from #120377 (1136f135dd), which reads the flag back from the persisted turn metadata. The suppressed case logs one DEBUG line naming platform and chat. Operator workaround until this lands: `platforms.slack.extra. ignore_other_user_mentions: true` drops peer-addressed messages before a turn exists. (cherry picked from commit 094439776ab898cccde303a1c2c911c8ab5bfb75)
This commit is contained in:
committed by
kshitij
parent
cda237464a
commit
b08bb5afb8
@@ -88,6 +88,9 @@ class MessageEvent:
|
||||
# May this event resolve gateway commands / control prompts? Proactive plugin events set False
|
||||
# so untrusted payload text stays conversational. Kept last for positional compat.
|
||||
allow_gateway_control: bool = True
|
||||
# Whether this inbound turn was addressed to this bot. False means the adapter admitted a
|
||||
# free-response or peer-addressed message, None means the adapter cannot determine it.
|
||||
reply_expected: Optional[bool] = None
|
||||
|
||||
# Process-local admission receipt, never routing metadata or execution acknowledgement.
|
||||
_gateway_accepted: bool = field(default=False, init=False, repr=False, compare=False)
|
||||
|
||||
@@ -7,7 +7,7 @@ not what should be persisted in conversation history.
|
||||
from __future__ import annotations
|
||||
|
||||
import unicodedata
|
||||
from typing import Any
|
||||
from typing import Any, Optional
|
||||
|
||||
# Exact whole-response markers meaning "the agent intentionally chose not to
|
||||
# reply". Keep small and explicit; arbitrary empty output remains an
|
||||
@@ -123,6 +123,11 @@ def is_machinery_display_kind(display_kind: Any) -> bool:
|
||||
return display_kind in MACHINERY_DISPLAY_KINDS
|
||||
|
||||
|
||||
def silence_allowed(display_kind: Any, reply_expected: Optional[bool] = None) -> bool:
|
||||
"""Whether a successful bare silence marker may remain silent for this turn."""
|
||||
return is_machinery_display_kind(display_kind) or reply_expected is False
|
||||
|
||||
|
||||
def is_partial_silence_marker(text: Any) -> bool:
|
||||
"""True while streamed ``text`` could still resolve to a silence marker.
|
||||
|
||||
|
||||
@@ -763,7 +763,7 @@ class GatewayStartupMixin:
|
||||
silence marker on a machinery turn, a muted diagnostic wake); else the text to send, with a
|
||||
human turn's bare silence marker replaced by the same notice the live path sends."""
|
||||
from gateway.platforms.base import _strip_media_directives
|
||||
from gateway.response_filters import is_intentional_silence_response, is_machinery_display_kind
|
||||
from gateway.response_filters import is_intentional_silence_response, silence_allowed
|
||||
from gateway.run import _sanitize_gateway_final_response
|
||||
from gateway.run_turn import _UNEXPECTED_SILENCE_REPLY
|
||||
from gateway.warning_notifications import diagnostic_turn_muted
|
||||
@@ -774,7 +774,10 @@ class GatewayStartupMixin:
|
||||
or (coerce_epoch(last.get("timestamp")) or 0) < started):
|
||||
return None
|
||||
prompt = next((m for m in reversed(visible) if m.get("role") == "user"), {})
|
||||
machinery = is_machinery_display_kind(prompt.get("display_kind"))
|
||||
machinery = silence_allowed(
|
||||
prompt.get("display_kind"),
|
||||
(prompt.get("display_metadata") or {}).get("reply_expected"),
|
||||
)
|
||||
if machinery:
|
||||
try: # the owning profile's display policy, as the adapter reads it at delivery
|
||||
scope = self._media_delivery_scope_for_source(origin)
|
||||
|
||||
@@ -1521,11 +1521,13 @@ class GatewayTurnMixin:
|
||||
self, agent_result, source, history, session_entry, session_key,
|
||||
_quick_key, run_generation, _run_start_session_id, _platform_name, _msg_start_time,
|
||||
persist_user_display_kind: Optional[str] = None,
|
||||
reply_expected: Optional[bool] = None,
|
||||
):
|
||||
"""Turn the raw agent result into the outbound text: sentinel/silence handling, response
|
||||
logging, resume-pending clear, empty-response normalization, and identity-guarded
|
||||
post-compression session_id propagation. Returns
|
||||
``(response, _intentional_silence, agent_messages)``."""
|
||||
from gateway.response_filters import silence_allowed
|
||||
from gateway.run import (
|
||||
_is_gateway_hidden_reasoning_incomplete_turn, _normalize_empty_agent_response,
|
||||
_sanitize_gateway_final_response, _should_clear_resume_pending_after_turn,
|
||||
@@ -1539,13 +1541,18 @@ class GatewayTurnMixin:
|
||||
# A queued (/queue) chain's TERMINAL turn owns the silence verdict, not the event that
|
||||
# opened the chain: an internal follow-up may go silent, a human one must not.
|
||||
_silence_kind = agent_result.get("queued_terminal_display_kind", persist_user_display_kind)
|
||||
if _intentional_silence and not is_machinery_display_kind(_silence_kind):
|
||||
if _intentional_silence and not silence_allowed(_silence_kind, reply_expected):
|
||||
logger.warning(
|
||||
"silence marker rejected on a user turn: platform=%s chat=%s",
|
||||
_platform_name, source.chat_id or "unknown",
|
||||
)
|
||||
_intentional_silence = False
|
||||
response = _UNEXPECTED_SILENCE_REPLY
|
||||
elif _intentional_silence and reply_expected is False:
|
||||
logger.debug(
|
||||
"silence marker suppressed on an unaddressed turn: platform=%s chat=%s",
|
||||
_platform_name, source.chat_id or "unknown",
|
||||
)
|
||||
|
||||
# "(empty)" = the model produced no visible content after exhausting all retries. One
|
||||
# text with the CLI explainer and the desktop (agent/turn_explainers.py) so the user
|
||||
@@ -2202,8 +2209,11 @@ class GatewayTurnMixin:
|
||||
persist_user_message=prepared.persist_user_message,
|
||||
persist_user_timestamp=prepared.persist_user_timestamp,
|
||||
persist_user_display_kind=prepared.persist_user_display_kind,
|
||||
reply_expected=getattr(event, "reply_expected", None),
|
||||
persist_user_display_metadata={
|
||||
"gateway_input_owner": prepared.persistence_owner, **diagnostic_metadata(event)},
|
||||
"gateway_input_owner": prepared.persistence_owner,
|
||||
"reply_expected": getattr(event, "reply_expected", None),
|
||||
**diagnostic_metadata(event)},
|
||||
message_type=event.message_type,
|
||||
scheduled_heartbeat=bool(getattr(event, "_heartbeat_session_id", None)),
|
||||
)
|
||||
@@ -2232,6 +2242,7 @@ class GatewayTurnMixin:
|
||||
agent_result, source, history, session_entry, session_key,
|
||||
_quick_key, run_generation, _run_start_session_id, _platform_name, _msg_start_time,
|
||||
persist_user_display_kind=prepared.persist_user_display_kind,
|
||||
reply_expected=getattr(event, "reply_expected", None),
|
||||
)
|
||||
response = self._hmwa_prepend_reasoning(agent_result, response, source, _intentional_silence)
|
||||
_footer_line = self._hmwa_runtime_footer_line(agent_result, source, _turn_seconds)
|
||||
@@ -3093,7 +3104,7 @@ class GatewayTurnMixin:
|
||||
|
||||
# The one-slot progress/holder containers shared with the callbacks are TurnContext defaults.
|
||||
turn_ctx = TurnContext(
|
||||
source=source, message=message, AIAgent=AIAgent, session_key=session_key,
|
||||
source=source, reply_expected=turn_params.pop("reply_expected", None), message=message, AIAgent=AIAgent, session_key=session_key,
|
||||
run_generation=run_generation, _cleanup_progress=_cleanup_progress,
|
||||
_run_still_current=self._run_still_current_fn(session_key, run_generation),
|
||||
progress_queue=queue.Queue() if disp.needs_progress_queue else None,
|
||||
@@ -3726,7 +3737,8 @@ class GatewayTurnMixin:
|
||||
)
|
||||
# Same silence predicate as the normal path, else this branch leaks the literal marker.
|
||||
if self._is_intentional_silence(_delivery_result, first_response):
|
||||
if is_machinery_display_kind(turn_ctx.persist_user_display_kind):
|
||||
from gateway.response_filters import silence_allowed
|
||||
if silence_allowed(turn_ctx.persist_user_display_kind, getattr(turn_ctx, "reply_expected", None)):
|
||||
logger.info(
|
||||
"Queued follow-up for session %s: suppressing intentional silence marker before continuing.",
|
||||
session_key or "?",
|
||||
@@ -4230,6 +4242,7 @@ class GatewayTurnMixin:
|
||||
persist_user_message: Optional[Any] = None, persist_user_timestamp: Optional[float] = None,
|
||||
persist_user_display_kind: Optional[str] = None, message_type: Optional[str] = None,
|
||||
persist_user_display_metadata: Optional[dict] = None,
|
||||
reply_expected: Optional[bool] = None,
|
||||
scheduled_heartbeat: bool = False,
|
||||
) -> Dict[str, Any]:
|
||||
"""Run the agent; returns the full run_conversation result dict.
|
||||
@@ -4266,6 +4279,7 @@ class GatewayTurnMixin:
|
||||
persist_user_message=persist_user_message,
|
||||
persist_user_timestamp=persist_user_timestamp,
|
||||
persist_user_display_kind=persist_user_display_kind,
|
||||
reply_expected=reply_expected,
|
||||
persist_user_display_metadata=persist_user_display_metadata,
|
||||
scheduled_heartbeat=scheduled_heartbeat,
|
||||
)
|
||||
|
||||
@@ -15,6 +15,7 @@ from typing import Any, Callable, List, Optional
|
||||
class TurnContext:
|
||||
# read-only turn identity / wiring
|
||||
source: Any = None
|
||||
reply_expected: Optional[bool] = None
|
||||
# Scheduled heartbeats are proactive work, not replies to the source message that
|
||||
# registered the watch. Their routine delivery surfaces stay quiet.
|
||||
scheduled_heartbeat: bool = False
|
||||
|
||||
@@ -85,6 +85,14 @@ _MODEL_PICKER_ACTION_IDS = (
|
||||
)
|
||||
|
||||
|
||||
def slack_reply_expected(*, is_one_to_one_dm: bool, is_mentioned: bool, is_command_text: bool) -> bool:
|
||||
"""Was this inbound message addressed to the bot? A 1:1 DM, an @mention of this bot or a
|
||||
command expects a reply, so the gateway must not let a bare silence marker vanish it. A message
|
||||
admitted only through a free-response channel, a thread follow-up or ``ignore_other_user_mentions:
|
||||
false`` may stay silent."""
|
||||
return bool(is_one_to_one_dm or is_mentioned or is_command_text)
|
||||
|
||||
|
||||
def _slack_unfurl_kwargs(extra: Optional[Dict[str, Any]]) -> Dict[str, bool]:
|
||||
"""Explicitly configured link-preview controls (omitted key = Slack default). String bools are
|
||||
coerced (config tooling persists YAML bools as strings); junk is dropped, NOT coerced to False,
|
||||
@@ -4662,7 +4670,9 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
event, text=text, original_text=original_text, command_probe_text=command_probe_text,
|
||||
is_command_text=is_command_text, channel_id=channel_id, team_id=team_id, ts=ts,
|
||||
user_id=user_id, thread_ts=thread_ts, is_dm=is_dm, media_urls=media_urls,
|
||||
media_types=media_types, media_text_inlined=media_text_inlined, channel_context=channel_context)
|
||||
media_types=media_types, media_text_inlined=media_text_inlined, channel_context=channel_context,
|
||||
reply_expected=slack_reply_expected(
|
||||
is_one_to_one_dm=is_one_to_one_dm, is_mentioned=is_mentioned, is_command_text=is_command_text))
|
||||
# React only when directly addressed; MPIMs are shared, so they need a
|
||||
# mention like any channel.
|
||||
if (is_one_to_one_dm or is_mentioned) and self._reactions_enabled():
|
||||
@@ -4682,7 +4692,7 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
self, event: dict, *, text: str, original_text: str, command_probe_text: str,
|
||||
is_command_text: bool, channel_id: str, team_id: str, ts: str, user_id: str,
|
||||
thread_ts: Optional[str], is_dm: bool, media_urls: List[str], media_types: List[str],
|
||||
media_text_inlined: List[bool], channel_context: Optional[str]) -> MessageEvent:
|
||||
media_text_inlined: List[bool], channel_context: Optional[str], reply_expected: Optional[bool] = None) -> MessageEvent:
|
||||
"""Resolve names, title the DM thread, and build the ``MessageEvent``. Commands are restored
|
||||
from canonical input: the parser needs the token at char zero and enrichment (blocks,
|
||||
unfurls, file text, history) must never mutate arguments."""
|
||||
@@ -4723,6 +4733,7 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
reply_to_message_id=thread_ts if thread_ts != ts else None,
|
||||
channel_prompt=self._channel_prompt_with_identity(channel_id, team_id),
|
||||
channel_context=channel_context,
|
||||
reply_expected=reply_expected,
|
||||
# thread_ts is the thread root, not an explicit reply (root is in channel_context).
|
||||
reply_to_text=None,
|
||||
auto_skill=resolve_channel_skills(self.config.extra, channel_id, None),
|
||||
|
||||
@@ -25,12 +25,13 @@ def _source():
|
||||
)
|
||||
|
||||
|
||||
def _event(*, internal: bool = False):
|
||||
def _event(*, internal: bool = False, reply_expected=None):
|
||||
return MessageEvent(
|
||||
text="side chatter",
|
||||
source=_source(),
|
||||
message_id="msg-42",
|
||||
internal=internal,
|
||||
reply_expected=reply_expected,
|
||||
)
|
||||
|
||||
|
||||
@@ -95,7 +96,8 @@ def test_failed_agent_result_never_counts_as_intentional_silence():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_human_turn_gets_a_visible_fallback_for_a_silence_marker(monkeypatch, tmp_path):
|
||||
@pytest.mark.parametrize("reply_expected", [None, True], ids=["adapter-unknown", "addressed"])
|
||||
async def test_human_turn_gets_a_visible_fallback_for_a_silence_marker(monkeypatch, tmp_path, reply_expected):
|
||||
runner = _runner(monkeypatch, tmp_path)
|
||||
runner._run_agent = AsyncMock(return_value={
|
||||
"final_response": "[SILENT]",
|
||||
@@ -111,12 +113,33 @@ async def test_human_turn_gets_a_visible_fallback_for_a_silence_marker(monkeypat
|
||||
})
|
||||
|
||||
response = await runner._handle_message_with_agent(
|
||||
_event(), _source(), "agent:main:telegram:group:-1001:12345", 1
|
||||
_event(reply_expected=reply_expected), _source(), "agent:main:telegram:group:-1001:12345", 1
|
||||
)
|
||||
|
||||
assert response and not is_intentional_silence_response(response)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unaddressed_human_turn_suppresses_silence_without_warning(monkeypatch, tmp_path, caplog):
|
||||
runner = _runner(monkeypatch, tmp_path)
|
||||
runner._run_agent = AsyncMock(return_value={
|
||||
"final_response": "[SILENT]",
|
||||
"messages": [{"role": "user", "content": "side chatter"},
|
||||
{"role": "assistant", "content": "[SILENT]"}],
|
||||
"tools": [], "history_offset": 0, "last_prompt_tokens": 0,
|
||||
"api_calls": 1, "failed": False,
|
||||
})
|
||||
with caplog.at_level("DEBUG"):
|
||||
response = await runner._handle_message_with_agent(
|
||||
_event(reply_expected=False), _source(), "agent:main:telegram:group:-1001:12345", 1
|
||||
)
|
||||
assert response == ""
|
||||
assert not any(record.levelname == "WARNING" and "silence marker" in record.message
|
||||
for record in caplog.records)
|
||||
assert any(record.levelname == "DEBUG" and "unaddressed" in record.message
|
||||
for record in caplog.records)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_internal_silence_token_suppresses_delivery_but_preserves_transcript(monkeypatch, tmp_path):
|
||||
runner = _runner(monkeypatch, tmp_path)
|
||||
|
||||
46
tests/gateway/test_slack_reply_expected.py
Normal file
46
tests/gateway/test_slack_reply_expected.py
Normal file
@@ -0,0 +1,46 @@
|
||||
"""The Slack adapter stamps ``reply_expected`` on the event it hands the gateway.
|
||||
|
||||
A message admitted only because of a free-response channel, a thread follow-up, or a mention of
|
||||
someone else carries ``reply_expected=False``; a 1:1 DM, an @mention of this bot, or a command
|
||||
carries ``True``. The gateway lets a bare silence marker stand on ``False`` and keeps the visible
|
||||
fallback on ``True`` (see tests/gateway/test_gateway_silence_tokens.py).
|
||||
"""
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from tests.gateway.test_slack_mention import _make_adapter
|
||||
|
||||
|
||||
async def _event(adapter, **kw):
|
||||
# _make_adapter builds the object without __init__; stub the network-backed resolvers only.
|
||||
adapter._resolve_user_name = AsyncMock(return_value="alice")
|
||||
adapter._resolve_channel_name = AsyncMock(return_value="general")
|
||||
adapter._humanize_user_mentions = AsyncMock(side_effect=lambda text, **_: text)
|
||||
adapter._channel_prompt_with_identity = lambda *_a, **_k: None
|
||||
base = dict(
|
||||
event={"user": "U1", "ts": "1.0", "channel": "C1", "text": "hi"}, text="hi", original_text="hi",
|
||||
command_probe_text="hi", is_command_text=False, channel_id="C1", team_id="T1", ts="1.0",
|
||||
user_id="U1", thread_ts=None, is_dm=False, media_urls=[], media_types=[],
|
||||
media_text_inlined=[], channel_context=None,
|
||||
)
|
||||
base.update(kw)
|
||||
return await adapter._build_message_event(**base)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("reply_expected", [True, False, None])
|
||||
async def test_build_message_event_carries_reply_expected(reply_expected):
|
||||
adapter = _make_adapter()
|
||||
ev = await _event(adapter, reply_expected=reply_expected)
|
||||
assert ev.reply_expected is reply_expected
|
||||
|
||||
|
||||
def test_reply_expected_rule_matches_addressing():
|
||||
"""The value the inbound handler computes: DM, mention of this bot, or a command."""
|
||||
from plugins.platforms.slack.adapter import slack_reply_expected
|
||||
assert slack_reply_expected(is_one_to_one_dm=True, is_mentioned=False, is_command_text=False) is True
|
||||
assert slack_reply_expected(is_one_to_one_dm=False, is_mentioned=True, is_command_text=False) is True
|
||||
assert slack_reply_expected(is_one_to_one_dm=False, is_mentioned=False, is_command_text=True) is True
|
||||
# Admitted via free channel / thread follow-up / peer mention with ignore_other_user_mentions off.
|
||||
assert slack_reply_expected(is_one_to_one_dm=False, is_mentioned=False, is_command_text=False) is False
|
||||
@@ -672,6 +672,7 @@ Set this to `true` in busy workspaces where Slack's default "the bot remembers t
|
||||
:::
|
||||
|
||||
:::tip When to use `ignore_other_user_mentions`
|
||||
If the model returns a bare silence marker for a message that was not addressed to this bot, the gateway keeps it silent. On a direct message or an explicit mention of the bot, the same marker produces the visible fallback notice.
|
||||
Set this to `true` when the bot follows busy threads (via thread auto-engagement or `free_response_channels`) and butts in on messages humans address to each other. It is a narrower tool than `strict_mention`: plain follow-ups in an engaged thread still get answers; only messages that open by @mentioning another person are skipped. **1:1 DMs are unaffected**; group DMs (MPIMs) and channels both apply it, matching the shared-surface policy below. Broadcast tokens (`@here`, `@channel`) and channel references address the room, not a person, so they are never skipped.
|
||||
:::
|
||||
|
||||
|
||||
Reference in New Issue
Block a user