fix(gateway): slash commands never interrupt a running agent (#12334)
Any recognized slash command now bypasses the Level-1 active-session guard instead of queueing + interrupting. A mid-run /model (or /reasoning, /voice, /insights, /title, /resume, /retry, /undo, /compress, /usage, /provider, /reload-mcp, /sethome, /reset) used to interrupt the agent AND get silently discarded by the slash-command safety net — zero-char response, dropped tool calls. Root cause: - Discord registers 41 native slash commands via tree.command(). - Only 14 were in ACTIVE_SESSION_BYPASS_COMMANDS. - The other ~15 user-facing ones fell through base.py:handle_message to the busy-session handler, which calls running_agent.interrupt() AND queues the text. - After the aborted run, gateway/run.py:9912 correctly identifies the queued text as a slash command and discards it — but the damage (interrupt + zero-char response) already happened. Fix: - should_bypass_active_session() now returns True for any resolvable slash command. ACTIVE_SESSION_BYPASS_COMMANDS stays as the subset with dedicated Level-2 handlers (documentation + tests). - gateway/run.py adds a catch-all after the dedicated handlers that returns a user-visible "agent busy — wait or /stop first" response for any other resolvable command. - Unknown text / file-path-like messages are unchanged — they still queue. Also: - gateway/platforms/discord.py logs the invoker identity on every slash command (user id + name + channel + guild) so future ghost-command reports can be triaged without guessing. Tests: - 15 new parametrized cases in test_command_bypass_active_session.py cover every previously-broken Discord slash command. - Existing tests for /stop, /new, /approve, /deny, /help, /status, /agents, /background, /steer, /update, /queue still pass. - test_steer.py's ACTIVE_SESSION_BYPASS_COMMANDS check still passes. Fixes #5057. Related: #6252, #10370, #4665.
This commit is contained in:
@@ -1933,6 +1933,24 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||||||
the "thinking..." indicator is replaced with that text; otherwise it
|
the "thinking..." indicator is replaced with that text; otherwise it
|
||||||
is deleted so the channel isn't cluttered.
|
is deleted so the channel isn't cluttered.
|
||||||
"""
|
"""
|
||||||
|
# Log the invoker so ghost-command reports can be triaged. Discord
|
||||||
|
# native slash invocations are always user-initiated (no bot can fire
|
||||||
|
# them), but mobile autocomplete / keyboard shortcuts / other users
|
||||||
|
# in the same channel are easy to miss in post-mortems.
|
||||||
|
try:
|
||||||
|
_user = interaction.user
|
||||||
|
_chan_id = getattr(interaction.channel, "id", None) or getattr(interaction, "channel_id", None)
|
||||||
|
logger.info(
|
||||||
|
"[Discord] slash '%s' invoked by user=%s id=%s channel=%s guild=%s",
|
||||||
|
command_text,
|
||||||
|
getattr(_user, "name", "?"),
|
||||||
|
getattr(_user, "id", "?"),
|
||||||
|
_chan_id,
|
||||||
|
getattr(interaction, "guild_id", None),
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
pass # logging must never block command dispatch
|
||||||
|
|
||||||
await interaction.response.defer(ephemeral=True)
|
await interaction.response.defer(ephemeral=True)
|
||||||
event = self._build_slash_event(interaction, command_text)
|
event = self._build_slash_event(interaction, command_text)
|
||||||
await self.handle_message(event)
|
await self.handle_message(event)
|
||||||
|
|||||||
@@ -2987,8 +2987,8 @@ class GatewayRunner:
|
|||||||
|
|
||||||
# Resolve the command once for all early-intercept checks below.
|
# Resolve the command once for all early-intercept checks below.
|
||||||
from hermes_cli.commands import (
|
from hermes_cli.commands import (
|
||||||
|
ACTIVE_SESSION_BYPASS_COMMANDS as _DEDICATED_HANDLERS,
|
||||||
resolve_command as _resolve_cmd_inner,
|
resolve_command as _resolve_cmd_inner,
|
||||||
should_bypass_active_session as _should_bypass_active_inner,
|
|
||||||
)
|
)
|
||||||
_evt_cmd = event.get_command()
|
_evt_cmd = event.get_command()
|
||||||
_cmd_def_inner = _resolve_cmd_inner(_evt_cmd) if _evt_cmd else None
|
_cmd_def_inner = _resolve_cmd_inner(_evt_cmd) if _evt_cmd else None
|
||||||
@@ -3123,11 +3123,9 @@ class GatewayRunner:
|
|||||||
if _cmd_def_inner and _cmd_def_inner.name == "background":
|
if _cmd_def_inner and _cmd_def_inner.name == "background":
|
||||||
return await self._handle_background_command(event)
|
return await self._handle_background_command(event)
|
||||||
|
|
||||||
# Gateway-handled info/control commands must never fall through to
|
# Gateway-handled info/control commands with dedicated
|
||||||
# the interrupt path. If they are queued as pending text, the
|
# running-agent handlers.
|
||||||
# slash-command safety net discards them before the user sees any
|
if _cmd_def_inner and _cmd_def_inner.name in _DEDICATED_HANDLERS:
|
||||||
# response.
|
|
||||||
if _cmd_def_inner and _should_bypass_active_inner(_cmd_def_inner.name):
|
|
||||||
if _cmd_def_inner.name == "help":
|
if _cmd_def_inner.name == "help":
|
||||||
return await self._handle_help_command(event)
|
return await self._handle_help_command(event)
|
||||||
if _cmd_def_inner.name == "commands":
|
if _cmd_def_inner.name == "commands":
|
||||||
@@ -3137,6 +3135,21 @@ class GatewayRunner:
|
|||||||
if _cmd_def_inner.name == "update":
|
if _cmd_def_inner.name == "update":
|
||||||
return await self._handle_update_command(event)
|
return await self._handle_update_command(event)
|
||||||
|
|
||||||
|
# Catch-all: any other recognized slash command reached the
|
||||||
|
# running-agent guard. Reject gracefully rather than falling
|
||||||
|
# through to interrupt + discard. Without this, commands
|
||||||
|
# like /model, /reasoning, /voice, /insights, /title,
|
||||||
|
# /resume, /retry, /undo, /compress, /usage, /provider,
|
||||||
|
# /reload-mcp, /sethome, /reset (all registered as Discord
|
||||||
|
# slash commands) would interrupt the agent AND get
|
||||||
|
# silently discarded by the slash-command safety net,
|
||||||
|
# producing a zero-char response. See #5057, #6252, #10370.
|
||||||
|
if _cmd_def_inner:
|
||||||
|
return (
|
||||||
|
f"⏳ Agent is running — `/{_cmd_def_inner.name}` can't run "
|
||||||
|
f"mid-turn. Wait for the current response or `/stop` first."
|
||||||
|
)
|
||||||
|
|
||||||
if event.message_type == MessageType.PHOTO:
|
if event.message_type == MessageType.PHOTO:
|
||||||
logger.debug("PRIORITY photo follow-up for session %s — queueing without interrupt", _quick_key[:20])
|
logger.debug("PRIORITY photo follow-up for session %s — queueing without interrupt", _quick_key[:20])
|
||||||
adapter = self.adapters.get(source.platform)
|
adapter = self.adapters.get(source.platform)
|
||||||
|
|||||||
@@ -260,10 +260,10 @@ GATEWAY_KNOWN_COMMANDS: frozenset[str] = frozenset(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
# Commands that must never be queued behind an active gateway session.
|
# Commands with explicit Level-2 running-agent handlers in gateway/run.py.
|
||||||
# These are explicit control/info commands handled by the gateway itself;
|
# Listed here for introspection / tests; semantically a subset of
|
||||||
# if they get queued as pending text, the safety net in gateway.run will
|
# "all resolvable commands" — which is the real bypass set (see
|
||||||
# discard them before they ever reach the user.
|
# should_bypass_active_session below).
|
||||||
ACTIVE_SESSION_BYPASS_COMMANDS: frozenset[str] = frozenset(
|
ACTIVE_SESSION_BYPASS_COMMANDS: frozenset[str] = frozenset(
|
||||||
{
|
{
|
||||||
"agents",
|
"agents",
|
||||||
@@ -285,9 +285,26 @@ ACTIVE_SESSION_BYPASS_COMMANDS: frozenset[str] = frozenset(
|
|||||||
|
|
||||||
|
|
||||||
def should_bypass_active_session(command_name: str | None) -> bool:
|
def should_bypass_active_session(command_name: str | None) -> bool:
|
||||||
"""Return True when a slash command must bypass active-session queuing."""
|
"""Return True for any resolvable slash command.
|
||||||
cmd = resolve_command(command_name) if command_name else None
|
|
||||||
return bool(cmd and cmd.name in ACTIVE_SESSION_BYPASS_COMMANDS)
|
Rationale: every gateway-registered slash command either has a
|
||||||
|
specific Level-2 handler in gateway/run.py (/stop, /new, /model,
|
||||||
|
/approve, etc.) or reaches the running-agent catch-all that returns
|
||||||
|
a "busy — wait or /stop first" response. In both paths the command
|
||||||
|
is dispatched, not queued.
|
||||||
|
|
||||||
|
Queueing is always wrong for a recognized slash command because the
|
||||||
|
safety net in gateway.run discards any command text that reaches
|
||||||
|
the pending queue — which meant a mid-run /model (or /reasoning,
|
||||||
|
/voice, /insights, /title, /resume, /retry, /undo, /compress,
|
||||||
|
/usage, /provider, /reload-mcp, /sethome, /reset) would silently
|
||||||
|
interrupt the agent AND get discarded, producing a zero-char
|
||||||
|
response. See issue #5057 / PRs #6252, #10370, #4665.
|
||||||
|
|
||||||
|
ACTIVE_SESSION_BYPASS_COMMANDS remains the subset of commands with
|
||||||
|
explicit Level-2 handlers; the rest fall through to the catch-all.
|
||||||
|
"""
|
||||||
|
return resolve_command(command_name) is not None if command_name else False
|
||||||
|
|
||||||
|
|
||||||
def _resolve_config_gates() -> set[str]:
|
def _resolve_config_gates() -> set[str]:
|
||||||
|
|||||||
@@ -268,6 +268,82 @@ class TestCommandBypassActiveSession:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Tests: non-bypass-set commands (no dedicated Level-2 handler) also bypass
|
||||||
|
# instead of interrupting + being discarded. Regression for the Discord
|
||||||
|
# ghost-slash-command bug where /model, /reasoning, /voice, /insights, /title,
|
||||||
|
# /resume, /retry, /undo, /compress, /usage, /provider, /reload-mcp,
|
||||||
|
# /sethome, /reset silently interrupted the running agent.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
class TestAllResolvableCommandsBypassGuard:
|
||||||
|
"""Every recognized slash command must bypass the Level-1 active-session
|
||||||
|
guard. Without this, commands the user fires mid-run interrupt the agent
|
||||||
|
AND get silently discarded by the slash-command safety net (zero-char
|
||||||
|
response)."""
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"command_text,canonical",
|
||||||
|
[
|
||||||
|
("/model claude-sonnet-4", "model"),
|
||||||
|
("/model", "model"),
|
||||||
|
("/reasoning high", "reasoning"),
|
||||||
|
("/personality default", "personality"),
|
||||||
|
("/voice on", "voice"),
|
||||||
|
("/insights 7", "insights"),
|
||||||
|
("/title my session", "title"),
|
||||||
|
("/resume yesterday", "resume"),
|
||||||
|
("/retry", "retry"),
|
||||||
|
("/undo", "undo"),
|
||||||
|
("/compress", "compress"),
|
||||||
|
("/usage", "usage"),
|
||||||
|
("/provider", "provider"),
|
||||||
|
("/reload-mcp", "reload-mcp"),
|
||||||
|
("/sethome", "sethome"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_command_bypasses_guard(self, command_text, canonical):
|
||||||
|
"""Any resolvable slash command bypasses instead of being queued."""
|
||||||
|
adapter = _make_adapter()
|
||||||
|
sk = _session_key()
|
||||||
|
adapter._active_sessions[sk] = asyncio.Event()
|
||||||
|
|
||||||
|
await adapter.handle_message(_make_event(command_text))
|
||||||
|
|
||||||
|
assert sk not in adapter._pending_messages, (
|
||||||
|
f"{command_text} was queued as pending — it should bypass the guard"
|
||||||
|
)
|
||||||
|
assert len(adapter.sent_responses) > 0, (
|
||||||
|
f"{command_text} produced no response — it should be dispatched, "
|
||||||
|
"not silently discarded"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_should_bypass_returns_true_for_every_registered_command(self):
|
||||||
|
"""Spot-check: the commands previously-broken on Discord all bypass."""
|
||||||
|
from hermes_cli.commands import should_bypass_active_session
|
||||||
|
|
||||||
|
for cmd in (
|
||||||
|
"model", "reasoning", "personality", "voice", "insights", "title",
|
||||||
|
"resume", "retry", "undo", "compress", "usage", "provider",
|
||||||
|
"reload-mcp", "sethome", "reset",
|
||||||
|
):
|
||||||
|
assert should_bypass_active_session(cmd) is True, (
|
||||||
|
f"/{cmd} must bypass the active-session guard"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_should_bypass_returns_false_for_unknown(self):
|
||||||
|
"""Unknown words don't bypass — they get queued as user text."""
|
||||||
|
from hermes_cli.commands import should_bypass_active_session
|
||||||
|
|
||||||
|
assert should_bypass_active_session("foobar") is False
|
||||||
|
assert should_bypass_active_session(None) is False
|
||||||
|
assert should_bypass_active_session("") is False
|
||||||
|
# A file path split on whitespace: '/path/to/file.py' -> 'path/to/file.py'
|
||||||
|
assert should_bypass_active_session("path/to/file.py") is False
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Tests: non-bypass messages still get queued
|
# Tests: non-bypass messages still get queued
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user