fix(toolbox-core)!: Simplify add_headers to make it sync in async client (#231)

* chore: Add unit test cases

* chore: Delint

* feat: Warn on insecure tool invocation with authentication

This change introduces a warning that is displayed immediately before a tool invocation if:
1. The invocation includes an authentication header.
2. The connection is being made over non-secure HTTP.

> [!IMPORTANT]
The purpose of this warning is to alert the user to the security risk of sending credentials over an unencrypted channel and to encourage the use of HTTPS.

* fix!: Warn about https only during tool initialization

* chore: Simplify add_headers to make it sync in async client

* chore: Fix unit tests

Author: anubhav756

packages/toolbox-core/src/toolbox_core/client.py

@@ -308,11 +308,11 @@ async def load_toolset(
 
         return tools
 
-    async def add_headers(
+    def add_headers(
         self, headers: Mapping[str, Union[Callable, Coroutine, str]]
     ) -> None:
         """
-        Asynchronously Add headers to be included in each request sent through this client.
+        Add headers to be included in each request sent through this client.
 
         Args:
             headers: Headers to include in each request sent through this client.

packages/toolbox-core/src/toolbox_core/sync_client.py

@@ -144,7 +144,7 @@ def load_toolset(
         if not self.__loop or not self.__thread:
             raise ValueError("Background loop or thread cannot be None.")
 
-        async_tools = asyncio.run_coroutine_threadsafe(coro, self.__loop).result()  # type: ignore
+        async_tools = asyncio.run_coroutine_threadsafe(coro, self.__loop).result()
         return [
             ToolboxSyncTool(async_tool, self.__loop, self.__thread)
             for async_tool in async_tools
@@ -154,18 +154,15 @@ def add_headers(
         self, headers: Mapping[str, Union[Callable, Coroutine, str]]
     ) -> None:
         """
-        Synchronously Add headers to be included in each request sent through this client.
+        Add headers to be included in each request sent through this client.
 
         Args:
             headers: Headers to include in each request sent through this client.
 
         Raises:
             ValueError: If any of the headers are already registered in the client.
         """
-        coro = self.__async_client.add_headers(headers)
-
-        # We have already created a new loop in the init method in case it does not already exist
-        asyncio.run_coroutine_threadsafe(coro, self.__loop).result()  # type: ignore
+        self.__async_client.add_headers(headers)
 
     def __enter__(self):
         """Enter the runtime context related to this client instance."""

packages/toolbox-core/tests/test_client.py

@@ -1443,7 +1443,7 @@ async def test_add_headers_success(
         )
 
         async with ToolboxClient(TEST_BASE_URL) as client:
-            await client.add_headers(static_header)
+            client.add_headers(static_header)
             assert client._ToolboxClient__client_headers == static_header
 
             tool = await client.load_tool(tool_name)

packages/toolbox-core/tests/test_sync_client.py

@@ -15,7 +15,7 @@
 
 import inspect
 from typing import Any, Callable, Mapping, Optional
-from unittest.mock import AsyncMock, patch
+from unittest.mock import AsyncMock, Mock, patch
 
 import pytest
 from aioresponses import CallbackResult, aioresponses
@@ -44,8 +44,12 @@ def sync_client_environment():
     # This ensures any client created will start a new loop/thread.
 
     # Ensure no loop/thread is running from a previous misbehaving test or setup
-    assert original_loop is None or not original_loop.is_running()
-    assert original_thread is None or not original_thread.is_alive()
+    if original_loop and original_loop.is_running():
+        original_loop.call_soon_threadsafe(original_loop.stop)
+        if original_thread and original_thread.is_alive():
+            original_thread.join(timeout=5)
+        ToolboxSyncClient._ToolboxSyncClient__loop = None
+        ToolboxSyncClient._ToolboxSyncClient__thread = None
 
     ToolboxSyncClient._ToolboxSyncClient__loop = None
     ToolboxSyncClient._ToolboxSyncClient__thread = None
@@ -408,20 +412,32 @@ def post_callback(url, **kwargs):
         result = tool(param1="test")
         assert result == expected_payload["result"]
 
-    @pytest.mark.usefixtures("sync_client_environment")
     def test_sync_add_headers_duplicate_fail(self):
-        """
-        Tests that adding a duplicate header via add_headers raises ValueError.
-        Manually create client to control initial headers.
-        """
+        """Tests that adding a duplicate header via add_headers raises ValueError (from async client)."""
         initial_headers = {"X-Initial-Header": "initial_value"}
+        mock_async_client = AsyncMock(spec=ToolboxClient)
 
-        with ToolboxSyncClient(TEST_BASE_URL, client_headers=initial_headers) as client:
-            with pytest.raises(
-                ValueError,
-                match="Client header\\(s\\) `X-Initial-Header` already registered",
-            ):
-                client.add_headers({"X-Initial-Header": "another_value"})
+        # Configure add_headers to simulate the ValueError from ToolboxClient
+        def mock_add_headers(headers):
+            # Simulate ToolboxClient's check
+            if "X-Initial-Header" in headers:
+                raise ValueError(
+                    "Client header(s) `X-Initial-Header` already registered"
+                )
+
+        mock_async_client.add_headers = Mock(side_effect=mock_add_headers)
+
+        with patch(
+            "toolbox_core.sync_client.ToolboxClient", return_value=mock_async_client
+        ):
+            with ToolboxSyncClient(
+                TEST_BASE_URL, client_headers=initial_headers
+            ) as client:
+                with pytest.raises(
+                    ValueError,
+                    match="Client header\\(s\\) `X-Initial-Header` already registered",
+                ):
+                    client.add_headers({"X-Initial-Header": "another_value"})
 
 
 class TestSyncAuth: