mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(slack): preserve typed command integrity across enrichment paths
Commands typed in Slack could be mangled by every enrichment layer the adapter applies to normal messages: - Block Kit / unfurl / attachment-notice / text-file injection could prepend or append content around a command, moving the command token away from character zero or polluting its arguments. Commands are now restored from canonical authored input after all enrichment (final is_command_text guard before MessageEvent construction). - @bot /cmd (typed slash behind a mention) was never classified as a command; the mention-strip branch now re-probes for both slash and bang forms. - The Slack Agent-view context label ([Slack app context: ...]) was prepended to command events too; now command-exempt. - Native slash payload arguments were strip()ed, destroying meaningful spacing inside/after arguments; only the command delimiter is nonsemantic now. - Slash payload thread identity (thread_ts/message_ts, top-level or nested in message/container) is preserved onto SessionSource so session-scoped commands hit the same thread session. - /queue and /steer queued fallbacks now propagate channel_context so a command that triggered first-entry thread backfill doesn't lose the history when re-queued. Adapted from PR #66310 to the post-#69320 channel_context design (thread history already rides MessageEvent.channel_context, never text).
This commit is contained in:
parent
e7ef57f8c3
commit
c2d306c4c2
5 changed files with 279 additions and 16 deletions
|
|
@ -10712,6 +10712,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
reply_to_is_own_message=event.reply_to_is_own_message,
|
||||
auto_skill=event.auto_skill,
|
||||
channel_prompt=event.channel_prompt,
|
||||
channel_context=event.channel_context,
|
||||
internal=event.internal,
|
||||
timestamp=event.timestamp,
|
||||
)
|
||||
|
|
@ -10741,6 +10742,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
source=event.source,
|
||||
message_id=event.message_id,
|
||||
channel_prompt=event.channel_prompt,
|
||||
channel_context=event.channel_context,
|
||||
)
|
||||
adapter._pending_messages[_quick_key] = queued_event
|
||||
return "Agent still starting — /steer queued for the next turn."
|
||||
|
|
@ -10763,6 +10765,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
source=event.source,
|
||||
message_id=event.message_id,
|
||||
channel_prompt=event.channel_prompt,
|
||||
channel_context=event.channel_context,
|
||||
)
|
||||
adapter._pending_messages[_quick_key] = queued_event
|
||||
return "No active agent — /steer queued for the next turn."
|
||||
|
|
|
|||
|
|
@ -4079,13 +4079,19 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
# Re-run command normalization against the canonical Slack text,
|
||||
# not the block-augmented agent text. Otherwise quoted/forwarded
|
||||
# rich-text payload can become accidental command arguments.
|
||||
# Handles both ``@bot !cmd`` (bang hidden behind the mention when
|
||||
# the first probe ran) and ``@bot /cmd`` (typed slash addressed
|
||||
# at the bot).
|
||||
mention_stripped = original_text.replace(f"<@{bot_uid}>", "").strip()
|
||||
command_text = _rewrite_known_bang_command(mention_stripped)
|
||||
if command_text != mention_stripped:
|
||||
if mention_stripped.startswith("/"):
|
||||
command_text = mention_stripped
|
||||
else:
|
||||
command_text = _rewrite_known_bang_command(mention_stripped)
|
||||
if command_text.startswith("/"):
|
||||
original_text = command_text
|
||||
text = command_text
|
||||
# Refresh command classification: the bang was hidden behind
|
||||
# the leading mention when the first probe ran.
|
||||
# Refresh command classification: the command token was
|
||||
# hidden behind the leading mention on the first probe.
|
||||
command_probe_text = command_text
|
||||
is_command_text = True
|
||||
# Register this thread so all future messages auto-trigger the bot.
|
||||
|
|
@ -4491,6 +4497,16 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
else:
|
||||
msg_type = MessageType.DOCUMENT
|
||||
|
||||
# Every enrichment path above (blocks, unfurls, attachment notices,
|
||||
# text-file injection, thread history) is deliberately allowed for
|
||||
# normal messages. Commands are restored from canonical authored
|
||||
# input only: the gateway parser requires the command token at
|
||||
# character zero, and enrichment must never mutate a command's
|
||||
# arguments.
|
||||
if is_command_text:
|
||||
text = command_probe_text
|
||||
msg_type = MessageType.COMMAND
|
||||
|
||||
# Resolve user display name (cached after first lookup)
|
||||
user_name = await self._resolve_user_name(
|
||||
user_id, chat_id=channel_id, team_id=team_id
|
||||
|
|
@ -4619,7 +4635,11 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
# the user switches views and would let stale context bleed into later
|
||||
# turns. The agent receives an inert label, never a fetched channel body.
|
||||
context_channel_id = agent_context.get("context_channel_id", "")
|
||||
if context_channel_id and context_channel_id != channel_id:
|
||||
if (
|
||||
context_channel_id
|
||||
and context_channel_id != channel_id
|
||||
and msg_event.message_type != MessageType.COMMAND
|
||||
):
|
||||
msg_event.text = (
|
||||
f"[Slack app context: user is viewing channel {context_channel_id}]\n\n"
|
||||
f"{msg_event.text}"
|
||||
|
|
@ -5762,7 +5782,8 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
message).
|
||||
"""
|
||||
slash_name = (command.get("command") or "").lstrip("/").strip()
|
||||
text = command.get("text", "").strip()
|
||||
raw_text = str(command.get("text") or "")
|
||||
text = raw_text
|
||||
user_id = command.get("user_id", "")
|
||||
channel_id = command.get("channel_id", "")
|
||||
team_id = command.get("team_id", "")
|
||||
|
|
@ -5775,29 +5796,32 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
# Legacy /hermes <subcommand> [args] routing + free-form questions.
|
||||
# Empty slash_name falls into this branch for backward compat
|
||||
# with any caller that didn't populate command["command"].
|
||||
legacy_text = raw_text.strip()
|
||||
from hermes_cli.commands import slack_subcommand_map
|
||||
|
||||
subcommand_map = slack_subcommand_map()
|
||||
subcommand_map["compact"] = "/compress"
|
||||
# Guard against whitespace-only text where ``text`` is truthy but
|
||||
# ``text.split()`` returns ``[]`` (e.g. user sends ``/hermes ``).
|
||||
parts = text.split() if text else []
|
||||
parts = legacy_text.split() if legacy_text else []
|
||||
first_word = parts[0] if parts else ""
|
||||
if first_word in subcommand_map:
|
||||
rest = text[len(first_word) :].strip()
|
||||
rest = legacy_text[len(first_word) :].strip()
|
||||
text = (
|
||||
f"{subcommand_map[first_word]} {rest}".strip()
|
||||
if rest
|
||||
else subcommand_map[first_word]
|
||||
)
|
||||
elif text:
|
||||
pass # Treat as a regular question
|
||||
elif legacy_text:
|
||||
text = legacy_text # Treat as a regular question
|
||||
else:
|
||||
text = "/help"
|
||||
else:
|
||||
# Native slash — /<slash_name> [args]. Route directly through the
|
||||
# gateway command dispatcher by prepending the slash.
|
||||
text = f"/{slash_name} {text}".strip()
|
||||
# gateway command dispatcher by prepending the slash. Only the
|
||||
# command delimiter is nonsemantic: preserve Slack's raw argument
|
||||
# payload, including meaningful internal/trailing spacing.
|
||||
text = f"/{slash_name}" if not raw_text else f"/{slash_name} {raw_text}"
|
||||
|
||||
# Slack slash commands can originate from DMs or shared channels.
|
||||
# Preserve DM semantics only for DM channel IDs; shared channels must
|
||||
|
|
|
|||
|
|
@ -175,6 +175,25 @@ async def test_queue_preserves_reply_context():
|
|||
assert queued.reply_to_author_name == "alice"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_queue_preserves_channel_context_backfill():
|
||||
"""A queued Slack thread command must retain first-entry history."""
|
||||
runner, adapter = _make_runner(_session_entry())
|
||||
sk = _running(runner)
|
||||
context = "[Thread context]\nAlice: earlier request"
|
||||
event = MessageEvent(
|
||||
text="/queue follow up",
|
||||
source=_make_source(),
|
||||
message_id="q-context",
|
||||
channel_context=context,
|
||||
)
|
||||
|
||||
result = await runner._handle_message(event)
|
||||
|
||||
assert result is not None and "queued" in result.lower()
|
||||
assert adapter._pending_messages[sk].channel_context == context
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_queue_no_text_no_media_returns_usage():
|
||||
runner, adapter = _make_runner(_session_entry())
|
||||
|
|
|
|||
|
|
@ -1534,6 +1534,157 @@ class TestBangPrefixCommands:
|
|||
# same thread.
|
||||
assert msg_event.source.thread_id == "1111111111.000001"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"authored_text", ["!queue --flag value ", "/queue --flag value "]
|
||||
)
|
||||
async def test_typed_command_preserves_trailing_argument_whitespace(
|
||||
self, adapter, authored_text
|
||||
):
|
||||
"""Canonicalization may remove composer padding, never argument bytes."""
|
||||
await adapter._handle_slack_message(self._make_event(authored_text))
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "/queue --flag value "
|
||||
assert msg_event.get_command_args() == "--flag value "
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_leading_space_bang_command_is_rewritten(self, adapter):
|
||||
"""Composer indentation before ``!cmd`` must not defeat the rewrite."""
|
||||
await adapter._handle_slack_message(self._make_event(" !queue follow up"))
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "/queue follow up"
|
||||
assert msg_event.message_type == MessageType.COMMAND
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_leading_space_slash_command_is_a_command(self, adapter):
|
||||
"""Users type `` /stop`` so Slack itself doesn't intercept the slash."""
|
||||
await adapter._handle_slack_message(self._make_event(" /stop"))
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "/stop"
|
||||
assert msg_event.message_type == MessageType.COMMAND
|
||||
assert msg_event.get_command() == "stop"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mentioned_bang_command_is_normalized(self, adapter):
|
||||
"""Mention stripping must not leave ``!command`` as ordinary text."""
|
||||
evt = self._make_event(
|
||||
"<@U_BOT> !reasoning xhigh",
|
||||
thread_ts="1111111111.000001",
|
||||
channel_type="channel",
|
||||
channel="C123",
|
||||
)
|
||||
await adapter._handle_slack_message(evt)
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "/reasoning xhigh"
|
||||
assert msg_event.message_type == MessageType.COMMAND
|
||||
assert msg_event.get_command() == "reasoning"
|
||||
assert msg_event.get_command_args() == "xhigh"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mentioned_unknown_bang_passes_through(self, adapter):
|
||||
"""``@bot !nice work`` is a casual message — must NOT be rewritten."""
|
||||
evt = self._make_event(
|
||||
"<@U_BOT> !nice work",
|
||||
channel_type="channel",
|
||||
channel="C123",
|
||||
)
|
||||
await adapter._handle_slack_message(evt)
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "!nice work"
|
||||
assert msg_event.message_type != MessageType.COMMAND
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mentioned_bang_command_ignores_rich_text_context(self, adapter):
|
||||
"""The combined mention + composer-block path retains exact arguments."""
|
||||
evt = self._make_event(
|
||||
"<@U_BOT> !reasoning xhigh",
|
||||
thread_ts="1111111111.000001",
|
||||
channel_type="channel",
|
||||
channel="C123",
|
||||
)
|
||||
evt["blocks"] = [
|
||||
{
|
||||
"type": "rich_text",
|
||||
"elements": [
|
||||
{
|
||||
"type": "rich_text_section",
|
||||
"elements": [
|
||||
{"type": "user", "user_id": "U_BOT"},
|
||||
{"type": "text", "text": " !reasoning xhigh"},
|
||||
],
|
||||
},
|
||||
{
|
||||
"type": "rich_text_quote",
|
||||
"elements": [
|
||||
{
|
||||
"type": "rich_text_section",
|
||||
"elements": [{"type": "text", "text": "quoted context"}],
|
||||
}
|
||||
],
|
||||
},
|
||||
],
|
||||
}
|
||||
]
|
||||
await adapter._handle_slack_message(evt)
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "/reasoning xhigh"
|
||||
assert "quoted context" not in msg_event.text
|
||||
assert msg_event.get_command_args() == "xhigh"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"enrichment",
|
||||
[
|
||||
{"attachments": [{"title": "Spec", "from_url": "https://example.com/spec", "text": "preview"}]},
|
||||
{"blocks": [{"type": "section", "text": {"type": "mrkdwn", "text": "UI metadata"}}]},
|
||||
],
|
||||
ids=["unfurl", "block-kit"],
|
||||
)
|
||||
async def test_bang_command_ignores_enrichment(self, adapter, enrichment):
|
||||
"""Rich Slack metadata is agent context, never command arguments."""
|
||||
event = self._make_event("!reasoning xhigh")
|
||||
event.update(enrichment)
|
||||
|
||||
await adapter._handle_slack_message(event)
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "/reasoning xhigh"
|
||||
assert msg_event.get_command_args() == "xhigh"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bang_command_ignores_app_view_context(self, adapter):
|
||||
"""Slack Agent-view metadata is prompt context, never command input."""
|
||||
event = self._make_event("!reasoning xhigh")
|
||||
event["app_context"] = {"channel_id": "C_VIEWED"}
|
||||
|
||||
await adapter._handle_slack_message(event)
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.text == "/reasoning xhigh"
|
||||
assert msg_event.get_command() == "reasoning"
|
||||
assert msg_event.get_command_args() == "xhigh"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_command_retains_app_view_context(self, adapter):
|
||||
"""Skipping app context is command-specific, not a loss of prompt context."""
|
||||
event = self._make_event("What is happening?")
|
||||
event["app_context"] = {"channel_id": "C_VIEWED"}
|
||||
|
||||
await adapter._handle_slack_message(event)
|
||||
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.message_type == MessageType.TEXT
|
||||
assert msg_event.text.startswith(
|
||||
"[Slack app context: user is viewing channel C_VIEWED]\n\n"
|
||||
)
|
||||
assert msg_event.text.endswith("What is happening?")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bang_queue_survives_first_thread_context_backfill(self, adapter):
|
||||
"""Backfill stays out of command text while remaining available."""
|
||||
|
|
@ -4579,6 +4730,59 @@ class TestSlashCommands:
|
|||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.text == "/model anthropic/claude-sonnet-4"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("thread_payload", "expected_thread_id"),
|
||||
[
|
||||
({"thread_ts": "1111111111.000001"}, "1111111111.000001"),
|
||||
({"message": {"thread_ts": "2222222222.000001"}}, "2222222222.000001"),
|
||||
({"container": {"thread_ts": "3333333333.000001"}}, "3333333333.000001"),
|
||||
({"message_ts": "4444444444.000001"}, "4444444444.000001"),
|
||||
({"container": {"message_ts": "5555555555.000001"}}, "5555555555.000001"),
|
||||
(
|
||||
{
|
||||
"message_ts": "fallback-message-ts",
|
||||
"message": {"thread_ts": "parent-thread-ts"},
|
||||
},
|
||||
"parent-thread-ts",
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_native_slash_preserves_thread_identity(
|
||||
self, adapter, thread_payload, expected_thread_id
|
||||
):
|
||||
"""Native Slack slash payload variants keep replies in their thread."""
|
||||
command = {
|
||||
"command": "/reasoning",
|
||||
"text": "xhigh",
|
||||
"user_id": "U1",
|
||||
"channel_id": "C1",
|
||||
**thread_payload,
|
||||
}
|
||||
|
||||
await adapter._handle_slash_command(command)
|
||||
|
||||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.source.thread_id == expected_thread_id
|
||||
assert msg.text == "/reasoning xhigh"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_native_slash_preserves_raw_argument_payload(self, adapter):
|
||||
"""Only the command delimiter is nonsemantic; raw Slack input stays intact."""
|
||||
raw_args = " --flag value "
|
||||
command = {
|
||||
"command": "/queue",
|
||||
"text": raw_args,
|
||||
"user_id": "U1",
|
||||
"channel_id": "C1",
|
||||
}
|
||||
|
||||
await adapter._handle_slash_command(command)
|
||||
|
||||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.text == f"/queue {raw_args}"
|
||||
assert msg.get_command_args() == "--flag value "
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_legacy_hermes_prefix_still_works(self, adapter):
|
||||
"""Backward compat: /hermes btw foo must still route to /btw foo.
|
||||
|
|
|
|||
|
|
@ -33,11 +33,12 @@ def _make_source() -> SessionSource:
|
|||
)
|
||||
|
||||
|
||||
def _make_event(text: str) -> MessageEvent:
|
||||
def _make_event(text: str, channel_context: str | None = None) -> MessageEvent:
|
||||
return MessageEvent(
|
||||
text=text,
|
||||
source=_make_source(),
|
||||
message_id="m1",
|
||||
channel_context=channel_context,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -140,13 +141,19 @@ async def test_steer_with_pending_sentinel_falls_back_to_queue():
|
|||
sk = build_session_key(_make_source())
|
||||
runner._running_agents[sk] = _AGENT_PENDING_SENTINEL
|
||||
|
||||
result = await runner._handle_message(_make_event("/steer wait up"))
|
||||
result = await runner._handle_message(
|
||||
_make_event("/steer wait up", channel_context="[Thread context]\nAlice: earlier request")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert "queued" in result.lower() or "starting" in result.lower()
|
||||
# The fallback put the text into the adapter's pending queue.
|
||||
# The fallback put the full turn payload into the adapter's pending queue.
|
||||
assert sk in adapter._pending_messages
|
||||
assert adapter._pending_messages[sk].text == "wait up"
|
||||
assert (
|
||||
adapter._pending_messages[sk].channel_context
|
||||
== "[Thread context]\nAlice: earlier request"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -161,13 +168,19 @@ async def test_steer_agent_without_steer_method_falls_back():
|
|||
running_agent = MagicMock(spec=[])
|
||||
runner._running_agents[sk] = running_agent
|
||||
|
||||
result = await runner._handle_message(_make_event("/steer fallback"))
|
||||
result = await runner._handle_message(
|
||||
_make_event("/steer fallback", channel_context="[Thread context]\nAlice: earlier request")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
# Must mention queueing since steer wasn't available
|
||||
assert "queued" in result.lower()
|
||||
assert sk in adapter._pending_messages
|
||||
assert adapter._pending_messages[sk].text == "fallback"
|
||||
assert (
|
||||
adapter._pending_messages[sk].channel_context
|
||||
== "[Thread context]\nAlice: earlier request"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue