mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-05 00:48:32 +00:00
fix(commands): reject invalid slash commands
This commit is contained in:
parent
287fd88fe4
commit
f45436b61d
@ -5,11 +5,14 @@ from __future__ import annotations
|
||||
import re
|
||||
from contextlib import AbstractContextManager
|
||||
from dataclasses import dataclass, field
|
||||
from difflib import get_close_matches
|
||||
from typing import TYPE_CHECKING, Any, Awaitable, Callable
|
||||
|
||||
from nanobot.bus.events import OutboundMessage
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.bus.events import InboundMessage, OutboundMessage
|
||||
from nanobot.bus.events import InboundMessage
|
||||
from nanobot.session.manager import Session
|
||||
from nanobot.utils.llm_runtime import LLMRuntime
|
||||
|
||||
@ -80,18 +83,21 @@ class CommandRouter:
|
||||
return normalize_command_text(text).lower() in self._priority
|
||||
|
||||
def is_dispatchable_command(self, text: str) -> bool:
|
||||
"""Check whether *text* matches any non-priority command tier (exact or prefix).
|
||||
"""Check whether *text* should be handled by non-priority dispatch.
|
||||
|
||||
Does NOT check priority tier.
|
||||
If this returns True, ``dispatch()`` is guaranteed to match a handler.
|
||||
Exact priority commands are handled separately. Recognized non-priority
|
||||
commands and invalid slash commands are dispatched here so malformed
|
||||
commands can be rejected instead of reaching the LLM.
|
||||
"""
|
||||
cmd = normalize_command_text(text).lower()
|
||||
if cmd in self._priority:
|
||||
return False
|
||||
if cmd in self._exact:
|
||||
return True
|
||||
for pfx, _ in self._prefix:
|
||||
if cmd.startswith(pfx):
|
||||
return True
|
||||
return False
|
||||
return cmd.startswith("/")
|
||||
|
||||
async def dispatch_priority(self, ctx: CommandContext) -> OutboundMessage | None:
|
||||
"""Dispatch a priority command. Called from run() without the lock."""
|
||||
@ -102,7 +108,7 @@ class CommandRouter:
|
||||
return None
|
||||
|
||||
async def dispatch(self, ctx: CommandContext) -> OutboundMessage | None:
|
||||
"""Try exact, then prefix handlers. Returns None if unhandled."""
|
||||
"""Try exact and prefix handlers, then reject invalid slash commands."""
|
||||
ctx.raw = normalize_command_text(ctx.raw)
|
||||
cmd = ctx.raw.lower()
|
||||
|
||||
@ -114,4 +120,51 @@ class CommandRouter:
|
||||
ctx.args = ctx.raw[len(pfx):]
|
||||
return await handler(ctx)
|
||||
|
||||
return None
|
||||
return self._invalid_command_response(ctx)
|
||||
|
||||
def _invalid_command_response(self, ctx: CommandContext) -> OutboundMessage | None:
|
||||
if not ctx.raw.startswith("/"):
|
||||
return None
|
||||
|
||||
entered = ctx.raw.split(maxsplit=1)[0]
|
||||
commands = self._registered_commands()
|
||||
canonical = commands.get(entered.lower())
|
||||
if canonical is not None:
|
||||
accepts_args = any(
|
||||
pfx.rstrip().lower() == entered.lower()
|
||||
for pfx, _ in self._prefix
|
||||
)
|
||||
if accepts_args:
|
||||
content = (
|
||||
f'Invalid command "{entered}". '
|
||||
'Use "/help" to list available commands.'
|
||||
)
|
||||
else:
|
||||
content = (
|
||||
f'Command "{canonical}" does not accept arguments. '
|
||||
f'Did you mean "{canonical}"?'
|
||||
)
|
||||
else:
|
||||
matches = get_close_matches(entered.lower(), commands, n=1, cutoff=0.6)
|
||||
if matches:
|
||||
content = (
|
||||
f'Unknown command "{entered}". '
|
||||
f'Did you mean "{commands[matches[0]]}"?'
|
||||
)
|
||||
else:
|
||||
content = (
|
||||
f'Unknown command "{entered}". '
|
||||
'Use "/help" to list available commands.'
|
||||
)
|
||||
|
||||
return OutboundMessage(
|
||||
channel=ctx.msg.channel,
|
||||
chat_id=ctx.msg.chat_id,
|
||||
content=content,
|
||||
metadata={**dict(ctx.msg.metadata or {}), "render_as": "text"},
|
||||
)
|
||||
|
||||
def _registered_commands(self) -> dict[str, str]:
|
||||
commands = [*self._priority, *self._exact]
|
||||
commands.extend(pfx.rstrip() for pfx, _ in self._prefix)
|
||||
return {command.lower(): command for command in commands if command}
|
||||
|
||||
@ -218,6 +218,47 @@ async def test_new_with_bot_suffix_does_not_persist_command(tmp_path: Path) -> N
|
||||
assert session.messages == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("content", "expected"),
|
||||
[
|
||||
("/neaw", 'Unknown command "/neaw". Did you mean "/new"?'),
|
||||
(
|
||||
"/status now",
|
||||
'Command "/status" does not accept arguments. Did you mean "/status"?',
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_invalid_slash_command_is_rejected_without_calling_provider(
|
||||
tmp_path: Path,
|
||||
content: str,
|
||||
expected: str,
|
||||
) -> None:
|
||||
loop = _make_full_loop(tmp_path)
|
||||
|
||||
response = await loop._process_message(
|
||||
InboundMessage(
|
||||
channel="websocket",
|
||||
sender_id="user",
|
||||
chat_id="chat-1",
|
||||
content=content,
|
||||
)
|
||||
)
|
||||
|
||||
assert response is not None
|
||||
assert response.content == expected
|
||||
loop.provider.chat_with_retry.assert_not_awaited()
|
||||
session = loop.sessions.get_or_create("websocket:chat-1")
|
||||
persisted = [
|
||||
(message["role"], message["content"], message.get("_command"))
|
||||
for message in session.messages
|
||||
]
|
||||
assert persisted == [
|
||||
("user", content, True),
|
||||
("assistant", response.content, True),
|
||||
]
|
||||
|
||||
|
||||
def test_clean_generated_title_strips_reasoning_tags() -> None:
|
||||
assert clean_generated_title("<think>reasoning</think> WebUI polish") == "WebUI polish"
|
||||
assert clean_generated_title("Title: <think> The user said hello") == ""
|
||||
|
||||
@ -70,9 +70,12 @@ class TestIsDispatchableCommand:
|
||||
assert router.is_dispatchable_command(" /new ")
|
||||
assert router.is_dispatchable_command(" /pairing list ")
|
||||
|
||||
def test_unknown_slash_command_not_matched(self, router: CommandRouter) -> None:
|
||||
assert not router.is_dispatchable_command("/unknown")
|
||||
assert not router.is_dispatchable_command("/foo bar")
|
||||
def test_invalid_slash_commands_match_for_explicit_rejection(
|
||||
self, router: CommandRouter,
|
||||
) -> None:
|
||||
assert router.is_dispatchable_command("/unknown")
|
||||
assert router.is_dispatchable_command("/foo bar")
|
||||
assert router.is_dispatchable_command("/status now")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
@ -183,6 +186,57 @@ class TestMidTurnCommandDispatchedDirectly:
|
||||
result = await router.dispatch(ctx)
|
||||
assert result is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unknown_command_suggests_close_match(
|
||||
self, router: CommandRouter, fake_loop: MagicMock, fake_msg: MagicMock,
|
||||
) -> None:
|
||||
fake_msg.content = "/neaw"
|
||||
ctx = CommandContext(
|
||||
msg=fake_msg, session=None,
|
||||
key="test:chat1", raw="/neaw", loop=fake_loop,
|
||||
)
|
||||
|
||||
result = await router.dispatch(ctx)
|
||||
|
||||
assert result is not None
|
||||
assert result.content == 'Unknown command "/neaw". Did you mean "/new"?'
|
||||
assert result.metadata["render_as"] == "text"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exact_command_with_arguments_suggests_valid_form(
|
||||
self, router: CommandRouter, fake_loop: MagicMock, fake_msg: MagicMock,
|
||||
) -> None:
|
||||
fake_msg.content = "/status now"
|
||||
ctx = CommandContext(
|
||||
msg=fake_msg, session=None,
|
||||
key="test:chat1", raw="/status now", loop=fake_loop,
|
||||
)
|
||||
|
||||
result = await router.dispatch(ctx)
|
||||
|
||||
assert result is not None
|
||||
assert result.content == (
|
||||
'Command "/status" does not accept arguments. Did you mean "/status"?'
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unknown_command_without_close_match_points_to_help(
|
||||
self, router: CommandRouter, fake_loop: MagicMock, fake_msg: MagicMock,
|
||||
) -> None:
|
||||
fake_msg.content = "/totally-unknown-command"
|
||||
ctx = CommandContext(
|
||||
msg=fake_msg, session=None,
|
||||
key="test:chat1", raw="/totally-unknown-command", loop=fake_loop,
|
||||
)
|
||||
|
||||
result = await router.dispatch(ctx)
|
||||
|
||||
assert result is not None
|
||||
assert result.content == (
|
||||
'Unknown command "/totally-unknown-command". '
|
||||
'Use "/help" to list available commands.'
|
||||
)
|
||||
|
||||
|
||||
class TestPairingCommandDispatch:
|
||||
"""Verify /pairing works via CommandRouter."""
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user