From f45436b61d85491d4dfefdcc523f2bb2ba8ad895 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Tue, 4 Aug 2026 17:03:13 +0800 Subject: [PATCH] fix(commands): reject invalid slash commands --- nanobot/command/router.py | 67 ++++++++++++++++++++--- tests/agent/test_loop_save_turn.py | 41 ++++++++++++++ tests/command/test_router_dispatchable.py | 60 +++++++++++++++++++- 3 files changed, 158 insertions(+), 10 deletions(-) diff --git a/nanobot/command/router.py b/nanobot/command/router.py index eb2939847..ef35ea29b 100644 --- a/nanobot/command/router.py +++ b/nanobot/command/router.py @@ -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} diff --git a/tests/agent/test_loop_save_turn.py b/tests/agent/test_loop_save_turn.py index f923de0c9..6ae74c675 100644 --- a/tests/agent/test_loop_save_turn.py +++ b/tests/agent/test_loop_save_turn.py @@ -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("reasoning WebUI polish") == "WebUI polish" assert clean_generated_title("Title: The user said hello") == "" diff --git a/tests/command/test_router_dispatchable.py b/tests/command/test_router_dispatchable.py index 837b55e23..697673ffe 100644 --- a/tests/command/test_router_dispatchable.py +++ b/tests/command/test_router_dispatchable.py @@ -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."""