chore: address review feedback round 2

Co-authored-by: BukeLy <19304666+BukeLy@users.noreply.github.com>

Author: Copilot

agent-sdk-client/consumer.py

@@ -56,17 +56,24 @@ async def process_message(message_data: dict) -> None:
         return
 
     if not config.is_command_allowed(message.text):
+        # Defensive guard: producer should already block non-whitelisted commands.
         logger.info(
-            "Skipping non-whitelisted command",
+            "Skipping non-whitelisted command (consumer fallback)",
             extra={
                 'chat_id': message.chat_id,
                 'message_id': message.message_id,
             },
         )
+        allowed = config.command_whitelist
+        if allowed:
+            allowed_list = "\n".join(allowed)
+            text = f"Unsupported command. Allowed commands:\n{allowed_list}"
+        else:
+            text = "Unsupported command."
         try:
             await bot.send_message(
                 chat_id=message.chat_id,
-                text="Unsupported command.",
+                text=text,
                 message_thread_id=message.message_thread_id,
                 reply_to_message_id=message.message_id,
             )

agent-sdk-client/handler.py

@@ -116,7 +116,7 @@ def _send_to_sqs_safe(sqs, queue_url: str, message_body: dict) -> bool:
             f"Unexpected error sending to SQS: {e}",
             extra={'exception_type': type(e).__name__},
         )
-            _send_metric('SQSError.Unexpected')
+        _send_metric('SQSError.Unexpected')
         return False
 
 

tests/test_command_whitelist.py

@@ -1,12 +1,15 @@
-import runpy
+import importlib.util
 from pathlib import Path
 
 import pytest
 
 CLIENT_CONFIG_PATH = Path(__file__).resolve().parent.parent / "agent-sdk-client" / "config.py"
-config_module = runpy.run_path(CLIENT_CONFIG_PATH)
-Config = config_module["Config"]
-load_command_whitelist = config_module["load_command_whitelist"]
+spec = importlib.util.spec_from_file_location("agent_sdk_client_config", CLIENT_CONFIG_PATH)
+config_module = importlib.util.module_from_spec(spec)
+assert spec.loader is not None
+spec.loader.exec_module(config_module)
+Config = config_module.Config
+load_command_whitelist = config_module.load_command_whitelist
 
 
 def test_load_command_whitelist(tmp_path):
@@ -43,3 +46,62 @@ def test_is_command_allowed(text, expected):
     )
 
     assert cfg.is_command_allowed(text) is expected
+
+
+def test_empty_whitelist_allows_commands():
+    cfg = Config(
+        telegram_token="",
+        agent_server_url="",
+        auth_token="",
+        queue_url="",
+        command_whitelist=[],
+    )
+    assert cfg.is_command_allowed("/anything") is False
+
+
+def test_none_text_treated_as_allowed():
+    cfg = Config(
+        telegram_token="",
+        agent_server_url="",
+        auth_token="",
+        queue_url="",
+        command_whitelist=["/allowed"],
+    )
+    assert cfg.is_command_allowed(None)
+
+
+def test_load_whitelist_non_string_entries(tmp_path):
+    config_path = tmp_path / "config.toml"
+    config_path.write_text(
+        """[white_list_commands]
+whitelist = ["/ok", 123, { a = 1 }]
+"""
+    )
+    assert load_command_whitelist(config_path) == ["/ok"]
+
+
+def test_load_whitelist_invalid_type_logs_and_returns_empty(tmp_path, caplog):
+    config_path = tmp_path / "config.toml"
+    config_path.write_text(
+        """[white_list_commands]
+whitelist = "not-a-list"
+"""
+    )
+    with caplog.at_level("WARNING"):
+        result = load_command_whitelist(config_path)
+    assert result == []
+    assert any("Command whitelist is not a list" in rec.message for rec in caplog.records)
+
+
+def test_load_whitelist_missing_file_returns_empty(tmp_path):
+    missing = tmp_path / "missing.toml"
+    assert load_command_whitelist(missing) == []
+
+
+def test_load_whitelist_malformed_toml_returns_empty(tmp_path, caplog):
+    config_path = tmp_path / "config.toml"
+    config_path.write_text("not = [ [")
+    with caplog.at_level("WARNING"):
+        result = load_command_whitelist(config_path)
+    assert result == []
+    assert any("Failed to load command whitelist" in rec.message for rec in caplog.records)