Skip to content

Commit f46c0d8

Browse files
committed
fix(tests): resolve unit test failures in tool and client wrappers
1 parent 3c99afa commit f46c0d8

File tree

5 files changed

+147
-133
lines changed

5 files changed

+147
-133
lines changed

packages/toolbox-adk/src/toolbox_adk/tool.py

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,33 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
from typing import Any, Dict, Optional, Callable, Awaitable
16-
from typing_extensions import override
15+
from typing import Any, Awaitable, Callable, Dict, Optional
1716

1817
import toolbox_core
19-
from toolbox_core.tool import ToolboxTool as CoreToolboxTool
20-
from google.adk.tools.base_tool import BaseTool
18+
from fastapi.openapi.models import (
19+
OAuth2,
20+
OAuthFlowAuthorizationCode,
21+
OAuthFlows,
22+
SecurityScheme,
23+
)
2124
from google.adk.agents.readonly_context import ReadonlyContext
22-
25+
from google.adk.auth.auth_credential import (
26+
AuthCredential,
27+
AuthCredentialTypes,
28+
OAuth2Auth,
29+
)
2330
from google.adk.auth.auth_tool import AuthConfig
24-
from google.adk.auth.auth_credential import AuthCredential, AuthCredentialTypes, OAuth2Auth
25-
from fastapi.openapi.models import SecurityScheme, OAuthFlows, OAuthFlowAuthorizationCode, OAuth2
31+
from google.adk.tools.base_tool import BaseTool
32+
from toolbox_core.tool import ToolboxTool as CoreToolboxTool
33+
from typing_extensions import override
2634

27-
from .credentials import CredentialConfig, CredentialType
2835
from .client import USER_TOKEN_CONTEXT_VAR
29-
36+
from .credentials import CredentialConfig, CredentialType
3037

3138

3239
class ToolboxContext:
3340
"""Context object passed to pre/post hooks."""
41+
3442
def __init__(self, arguments: Dict[str, Any], tool_context: ReadonlyContext):
3543
self.arguments = arguments
3644
self.tool_context = tool_context
@@ -59,15 +67,18 @@ def __init__(
5967
"""
6068
# We act as a proxy.
6169
# We need to extract metadata from the core tool to satisfy BaseTool's contract.
62-
70+
6371
name = getattr(core_tool, "__name__", "unknown_tool")
64-
description = getattr(core_tool, "__doc__", "No description provided.") or "No description provided."
65-
72+
description = (
73+
getattr(core_tool, "__doc__", "No description provided.")
74+
or "No description provided."
75+
)
76+
6677
super().__init__(
6778
name=name,
6879
description=description,
6980
# We pass empty custom_metadata or whatever is needed
70-
custom_metadata={}
81+
custom_metadata={},
7182
)
7283
self._core_tool = core_tool
7384
self._pre_hook = pre_hook
@@ -82,30 +93,32 @@ async def run_async(
8293
) -> Any:
8394
# Create context
8495
ctx = ToolboxContext(arguments=args, tool_context=tool_context)
85-
96+
8697
# 1. Pre-hook
8798
if self._pre_hook:
8899
await self._pre_hook(ctx)
89-
100+
90101
# 2. ADK Auth Integration (3LO)
91102
# Check if USER_IDENTITY is configured
92103
reset_token = None
93-
104+
94105
if self._auth_config and self._auth_config.type == CredentialType.USER_IDENTITY:
95106
if not self._auth_config.client_id or not self._auth_config.client_secret:
96107
raise ValueError("USER_IDENTITY requires client_id and client_secret")
97-
108+
98109
# Construct ADK AuthConfig
99-
scopes = self._auth_config.scopes or ["https://www.googleapis.com/auth/cloud-platform"]
110+
scopes = self._auth_config.scopes or [
111+
"https://www.googleapis.com/auth/cloud-platform"
112+
]
100113
scope_dict = {s: "" for s in scopes}
101-
114+
102115
auth_config_adk = AuthConfig(
103116
auth_scheme=OAuth2(
104117
flows=OAuthFlows(
105118
authorizationCode=OAuthFlowAuthorizationCode(
106119
authorizationUrl="https://accounts.google.com/o/oauth2/auth",
107120
tokenUrl="https://oauth2.googleapis.com/token",
108-
scopes=scope_dict
121+
scopes=scope_dict,
109122
)
110123
)
111124
),
@@ -114,9 +127,9 @@ async def run_async(
114127
oauth2=OAuth2Auth(
115128
client_id=self._auth_config.client_id,
116129
client_secret=self._auth_config.client_secret,
117-
scopes=scopes
118-
)
119-
)
130+
scopes=scopes,
131+
),
132+
),
120133
)
121134

122135
# Check if we already have credentials from a previous exchange
@@ -140,19 +153,21 @@ async def run_async(
140153
# Actually, strictly we should probably request credential if get_auth_response returns nothing
141154
# but get_auth_response typically handles the lookup.
142155
# If exception is unrelated, raise.
143-
if "credential" in str(e).lower() or isinstance(e, ValueError): # Loose check, but safest is to re-raise
144-
raise e
156+
if "credential" in str(e).lower() or isinstance(
157+
e, ValueError
158+
): # Loose check, but safest is to re-raise
159+
raise e
145160
# Fallback to request logic if it was a lookup error?
146161
tool_context.request_credential(auth_config_adk)
147162
return None
148163

149164
try:
150165
# Execute the core tool
151166
result = await self._core_tool(**ctx.arguments)
152-
167+
153168
ctx.result = result
154169
return result
155-
170+
156171
except Exception as e:
157172
ctx.error = e
158173
raise e
@@ -162,13 +177,13 @@ async def run_async(
162177
if self._post_hook:
163178
await self._post_hook(ctx)
164179

165-
def bind_params(self, bounded_params: Dict[str, Any]) -> 'ToolboxTool':
180+
def bind_params(self, bounded_params: Dict[str, Any]) -> "ToolboxTool":
166181
"""Allows runtime binding of parameters, delegating to core tool."""
167182
new_core_tool = self._core_tool.bind_params(bounded_params)
168183
# Return a new wrapper
169184
return ToolboxTool(
170-
core_tool=new_core_tool,
171-
pre_hook=self._pre_hook,
185+
core_tool=new_core_tool,
186+
pre_hook=self._pre_hook,
172187
post_hook=self._post_hook,
173-
auth_config=self._auth_config
188+
auth_config=self._auth_config,
174189
)

packages/toolbox-adk/src/toolbox_adk/toolset.py

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,17 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
from typing import Any, Dict, List, Optional, Mapping, Union, Callable, Awaitable
16-
from typing_extensions import override
15+
from typing import Any, Awaitable, Callable, Dict, List, Mapping, Optional, Union
1716

17+
from google.adk.agents.readonly_context import ReadonlyContext
1818
from google.adk.tools.base_tool import BaseTool
1919
from google.adk.tools.base_toolset import BaseToolset
20-
from google.adk.agents.readonly_context import ReadonlyContext
20+
from typing_extensions import override
2121

2222
from .client import ToolboxClient
2323
from .credentials import CredentialConfig
24-
from .tool import ToolboxTool, ToolboxContext
24+
from .tool import ToolboxContext, ToolboxTool
25+
2526

2627
class ToolboxToolset(BaseToolset):
2728
"""
@@ -34,11 +35,13 @@ def __init__(
3435
toolset_name: Optional[str] = None,
3536
tool_names: Optional[List[str]] = None,
3637
credentials: Optional[CredentialConfig] = None,
37-
additional_headers: Optional[Dict[str, Union[str, Callable[[], str], Callable[[], Awaitable[str]]]]] = None,
38+
additional_headers: Optional[
39+
Dict[str, Union[str, Callable[[], str], Callable[[], Awaitable[str]]]]
40+
] = None,
3841
bound_params: Optional[Mapping[str, Union[Callable[[], Any], Any]]] = None,
3942
pre_hook: Optional[Callable[[ToolboxContext], Awaitable[None]]] = None,
4043
post_hook: Optional[Callable[[ToolboxContext], Awaitable[None]]] = None,
41-
**kwargs
44+
**kwargs,
4245
):
4346
"""
4447
Args:
@@ -56,7 +59,7 @@ def __init__(
5659
server_url=server_url,
5760
credentials=credentials,
5861
additional_headers=additional_headers,
59-
**kwargs
62+
**kwargs,
6063
)
6164
self._toolset_name = toolset_name
6265
self._tool_names = tool_names
@@ -69,38 +72,36 @@ async def get_tools(
6972
self, readonly_context: Optional[ReadonlyContext] = None
7073
) -> List[BaseTool]:
7174
"""Loads tools from the toolbox server and wraps them."""
72-
# Note: We don't close the client after get_tools because tools might need it
75+
# Note: We don't close the client after get_tools because tools might need it
7376
# (though currently tools are self-contained http-wise if they don't share session state).
7477
# Actually toolbox-core tools have their own client reference or use a shared one?
75-
# toolbox_core.ToolboxClient.load_tool returns a tool which...
78+
# toolbox_core.ToolboxClient.load_tool returns a tool which...
7679
# wait, toolbox_core tools make HTTP calls. Do they hold a reference to the client session?
7780
# Yes, toolbox_core 0.1.0+ tools usually have an internal `client` or `session`.
7881
# So we must keep self._client alive or ensuring it's not closed prematurely.
79-
82+
8083
tools = []
8184
if self._toolset_name:
8285
core_tools = await self._client.load_toolset(
83-
self._toolset_name,
84-
bound_params=self._bound_params
86+
self._toolset_name, bound_params=self._bound_params
8587
)
8688
tools.extend(core_tools)
8789

8890
if self._tool_names:
8991
for name in self._tool_names:
9092
core_tool = await self._client.load_tool(
91-
name,
92-
bound_params=self._bound_params
93+
name, bound_params=self._bound_params
9394
)
9495
tools.append(core_tool)
95-
96+
9697
# Wrap all core tools in ToolboxTool
9798
return [
9899
ToolboxTool(
99-
core_tool=t,
100-
pre_hook=self._pre_hook,
100+
core_tool=t,
101+
pre_hook=self._pre_hook,
101102
post_hook=self._post_hook,
102-
auth_config=self._client.credential_config
103-
)
103+
auth_config=self._client.credential_config,
104+
)
104105
for t in tools
105106
]
106107

packages/toolbox-adk/tests/unit/test_client.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,15 @@ def test_init_no_auth(self, mock_core_client):
2727
creds = CredentialStrategy.TOOLBOX_IDENTITY()
2828
client = ToolboxClient("http://server", credentials=creds)
2929

30-
mock_core_client.assert_called_with(
31-
server_url="http://server", client_headers={}
32-
)
30+
mock_core_client.assert_called_with(url="http://server", client_headers={})
3331

3432
@patch("toolbox_adk.client.toolbox_core.ToolboxClient")
3533
def test_init_manual_token(self, mock_core_client):
3634
creds = CredentialStrategy.MANUAL_TOKEN("abc")
3735
client = ToolboxClient("http://server", credentials=creds)
3836

3937
mock_core_client.assert_called_with(
40-
server_url="http://server", client_headers={"Authorization": "Bearer abc"}
38+
url="http://server", client_headers={"Authorization": "Bearer abc"}
4139
)
4240

4341
@patch("toolbox_adk.client.toolbox_core.ToolboxClient")
@@ -49,7 +47,7 @@ def test_init_additional_headers(self, mock_core_client):
4947
)
5048

5149
mock_core_client.assert_called_with(
52-
server_url="http://server", client_headers={"X-Custom": "Val"}
50+
url="http://server", client_headers={"X-Custom": "Val"}
5351
)
5452

5553
@patch("toolbox_adk.client.toolbox_core.ToolboxClient")

0 commit comments

Comments
 (0)