mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-08-25 12:00:11 +00:00
fix(command_manager): prevent a disabled command from swallowing a keyword preventing an alias from firing
Removed redundant logging for command execution and added detailed debug logging for cases where a command cannot execute due to soft rejections. This allows for better tracking of command flow and ensures that keyword matching continues for subsequent commands. Updated the test_command to include config aliases in keyword matching, enhancing its flexibility.
This commit is contained in:
@@ -2009,8 +2009,6 @@ class CommandManager:
|
||||
# This command was already handled by keyword matching
|
||||
continue
|
||||
|
||||
self.logger.info(f"Command '{command_name}' matched, executing")
|
||||
|
||||
# Check if we should queue instead of reject (for global cooldowns near expiring)
|
||||
should_queue, remaining = self._should_queue_command(command, message)
|
||||
if should_queue and self._queue_command(command, message, remaining):
|
||||
@@ -2054,7 +2052,17 @@ class CommandManager:
|
||||
await self.send_response(message, error_msg)
|
||||
response_sent = True
|
||||
|
||||
# Record command execution in stats database (even if it failed checks)
|
||||
# Soft rejection (e.g. enabled=false): do not claim the keyword.
|
||||
# Matches check_keywords(), which continues so another command's
|
||||
# alias can handle the same trigger (e.g. test aliases=path with
|
||||
# Path_Command disabled).
|
||||
if not response_sent:
|
||||
self.logger.debug(
|
||||
f"Command '{command_name}' matched but cannot execute; trying next"
|
||||
)
|
||||
continue
|
||||
|
||||
# Record command execution in stats database (hard rejection with user feedback)
|
||||
if 'stats' in self.commands:
|
||||
stats_command = self.commands['stats']
|
||||
if stats_command:
|
||||
@@ -2062,6 +2070,8 @@ class CommandManager:
|
||||
|
||||
return
|
||||
|
||||
self.logger.info(f"Command '{command_name}' matched, executing")
|
||||
|
||||
# Check network connectivity for commands that require internet
|
||||
if command.requires_internet:
|
||||
has_internet = await self._check_internet_cached_async()
|
||||
|
||||
@@ -109,7 +109,8 @@ class TestCommand(BaseCommand):
|
||||
def matches_keyword(self, message: MeshMessage) -> bool:
|
||||
"""Override to implement special test keyword matching with optional phrase.
|
||||
|
||||
Matches 'test', 't', 'test <phrase>', or 't <phrase>'.
|
||||
Matches 'test', 't', 'test <phrase>', or 't <phrase>', plus any
|
||||
config ``aliases`` loaded into ``self.keywords``.
|
||||
|
||||
Args:
|
||||
message: The message to check.
|
||||
@@ -138,7 +139,8 @@ class TestCommand(BaseCommand):
|
||||
phrase = content[2:].strip() # Get everything after "t " and strip whitespace
|
||||
return bool(phrase) # Make sure there's actually a phrase
|
||||
|
||||
return False
|
||||
# Config aliases (e.g. aliases = path, p) live in self.keywords via BaseCommand
|
||||
return super().matches_keyword(message)
|
||||
|
||||
DEFAULT_FORMAT = "ack @[{sender}]{phrase_part} | {connection_info} | Received at: {timestamp}"
|
||||
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Regression: disabled built-in must not block another command's alias.
|
||||
|
||||
Reproduces the report where [Test_Command] aliases = path, p with
|
||||
[Path_Command] enabled = false still claimed !path and sent no reply.
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, Mock
|
||||
|
||||
import pytest
|
||||
|
||||
from modules.commands.path_command import PathCommand
|
||||
from modules.commands.test_command import TestCommand as MeshTestCommand
|
||||
from tests.conftest import mock_message
|
||||
from tests.test_command_manager import make_manager
|
||||
from tests.unit.test_command_path_byte_gating import _base_bot
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_test_command_aliases_match_path_and_p():
|
||||
bot = _base_bot()
|
||||
bot.config.add_section("Test_Command")
|
||||
bot.config.set("Test_Command", "enabled", "true")
|
||||
bot.config.set("Test_Command", "aliases", "path, p")
|
||||
|
||||
cmd = MeshTestCommand(bot)
|
||||
assert "path" in cmd.keywords
|
||||
assert "p" in cmd.keywords
|
||||
assert cmd.matches_keyword(mock_message(content="!path", is_dm=True)) is True
|
||||
assert cmd.matches_keyword(mock_message(content="path", is_dm=True)) is True
|
||||
assert cmd.matches_keyword(mock_message(content="p", is_dm=True)) is True
|
||||
assert cmd.matches_keyword(mock_message(content="test", is_dm=True)) is True
|
||||
assert cmd.matches_keyword(mock_message(content="ping", is_dm=True)) is False
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_check_keywords_prefers_test_alias_when_path_disabled():
|
||||
bot = _base_bot()
|
||||
bot.config.add_section("Path_Command")
|
||||
bot.config.set("Path_Command", "enabled", "false")
|
||||
bot.config.add_section("Test_Command")
|
||||
bot.config.set("Test_Command", "enabled", "true")
|
||||
bot.config.set("Test_Command", "aliases", "path, p")
|
||||
bot.config.set("Test_Command", "response_format", "ack-from-test")
|
||||
|
||||
path_cmd = PathCommand(bot)
|
||||
test_cmd = MeshTestCommand(bot)
|
||||
# path before test — same claim order as the user report
|
||||
manager = make_manager(bot, commands={"path": path_cmd, "test": test_cmd})
|
||||
|
||||
matches = manager.check_keywords(mock_message(content="!path", is_dm=True))
|
||||
assert matches == [("test", "ack-from-test")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.unit
|
||||
async def test_execute_commands_skips_disabled_and_runs_next():
|
||||
bot = _base_bot()
|
||||
bot.config.add_section("Path_Command")
|
||||
bot.config.set("Path_Command", "enabled", "false")
|
||||
|
||||
path_cmd = PathCommand(bot)
|
||||
path_cmd.should_execute = Mock(return_value=True)
|
||||
path_cmd.get_response_format = Mock(return_value=None)
|
||||
path_cmd.execute = AsyncMock(return_value=True)
|
||||
|
||||
other = MagicMock()
|
||||
other.is_channel_allowed = Mock(return_value=True)
|
||||
other.should_execute = Mock(return_value=True)
|
||||
other.get_response_format = Mock(return_value=None)
|
||||
other.can_execute_now = Mock(return_value=True)
|
||||
other.requires_internet = False
|
||||
other.cooldown_seconds = 0
|
||||
other.execute = AsyncMock(return_value=True)
|
||||
other._record_execution = Mock()
|
||||
other.last_response = None
|
||||
|
||||
manager = make_manager(bot, commands={"path": path_cmd, "other": other})
|
||||
manager.send_response = AsyncMock(return_value=True)
|
||||
|
||||
await manager.execute_commands(mock_message(content="!path", is_dm=True))
|
||||
|
||||
path_cmd.execute.assert_not_called()
|
||||
other.execute.assert_awaited_once()
|
||||
Reference in New Issue
Block a user