fix: avoid double-execution of sync FunctionTools returning None

_call_tool_in_thread_pool used None as a sentinel to distinguish
"FunctionTool ran successfully" from "non-FunctionTool sync tool,
needs async fallback". When a sync FunctionTool's function legitimately
returned None, the sentinel check fell through to tool.run_async() and
re-invoked the underlying function.

Restructure the dispatch so the sync-FunctionTool path returns directly
and the non-FunctionTool sync path falls through explicitly, removing
the ambiguous sentinel.

Close #5284

Co-authored-by: George Weale <gweale@google.com>
PiperOrigin-RevId: 905258186

Author: GWeale

src/google/adk/flows/llm_flows/functions.py

@@ -145,9 +145,9 @@ async def _call_tool_in_thread_pool(
   executor = _get_tool_thread_pool(max_workers)
 
   if _is_sync_tool(tool):
-    # For sync FunctionTool, call the underlying function directly
-    def run_sync_tool():
-      if isinstance(tool, FunctionTool):
+    if isinstance(tool, FunctionTool):
+      # For sync FunctionTool, call the underlying function directly.
+      def run_sync_tool():
         args_to_call = tool._preprocess_args(args)
         signature = inspect.signature(tool.func)
         valid_params = {param for param in signature.parameters}
@@ -157,15 +157,10 @@ def run_sync_tool():
             k: v for k, v in args_to_call.items() if k in valid_params
         }
         return tool.func(**args_to_call)
-      else:
-        # For other sync tool types, we can't easily run them in thread pool
-        return None
 
-    result = await loop.run_in_executor(
-        executor, lambda: ctx.run(run_sync_tool)
-    )
-    if result is not None:
-      return result
+      return await loop.run_in_executor(
+          executor, lambda: ctx.run(run_sync_tool)
+      )
   else:
     # For async tools, run them in a new event loop in a background thread.
     # This helps when async functions contain blocking I/O (common user mistake)
@@ -178,7 +173,7 @@ def run_async_tool_in_new_loop():
         executor, lambda: ctx.run(run_async_tool_in_new_loop)
     )
 
-  # Fall back to normal async execution for non-FunctionTool sync tools
+  # Fall back to normal async execution for non-FunctionTool sync tools.
   return await tool.run_async(args=args, tool_context=tool_context)
 
 

tests/unittests/flows/llm_flows/test_functions_thread_pool.py

@@ -25,9 +25,12 @@
 from google.adk.flows.llm_flows.functions import _call_tool_in_thread_pool
 from google.adk.flows.llm_flows.functions import _get_tool_thread_pool
 from google.adk.flows.llm_flows.functions import _is_sync_tool
+from google.adk.tools.base_tool import BaseTool
 from google.adk.tools.function_tool import FunctionTool
+from google.adk.tools.set_model_response_tool import SetModelResponseTool
 from google.adk.tools.tool_context import ToolContext
 from google.genai import types
+from pydantic import BaseModel
 import pytest
 
 from ... import testing_utils
@@ -76,8 +79,6 @@ async def async_gen_func(x: int):
 
   def test_tool_without_func_returns_false(self):
     """Test that a tool without func attribute returns False."""
-    from google.adk.tools.base_tool import BaseTool
-
     tool = BaseTool(name='test', description='test tool')
     assert _is_sync_tool(tool) is False
 
@@ -398,6 +399,99 @@ async def async_func() -> dict[str, str]:
 
     assert result == {'value': 'main_thread_value'}
 
+  @pytest.mark.asyncio
+  async def test_sync_tool_returning_none_runs_exactly_once(self):
+    """Regression test for issue #5284.
+
+    A sync FunctionTool whose underlying function returns None must not
+    be re-invoked through the run_async fallback path.
+    """
+    call_count = 0
+
+    def side_effect_only_func() -> None:
+      nonlocal call_count
+      call_count += 1
+
+    tool = FunctionTool(side_effect_only_func)
+    model = testing_utils.MockModel.create(responses=[])
+    agent = Agent(name='test_agent', model=model, tools=[tool])
+    invocation_context = await testing_utils.create_invocation_context(
+        agent=agent, user_content=''
+    )
+    tool_context = ToolContext(
+        invocation_context=invocation_context,
+        function_call_id='test_id',
+    )
+
+    result = await _call_tool_in_thread_pool(tool, {}, tool_context)
+
+    assert result is None
+    assert call_count == 1
+
+  @pytest.mark.asyncio
+  async def test_non_function_tool_sync_falls_back_to_run_async(self):
+    """Sync tools that aren't FunctionTool subclasses go through run_async.
+
+    Covers the fall-through path used by tools like SetModelResponseTool
+    that have a sync ``func`` attribute but aren't FunctionTool instances.
+    """
+    run_async_call_count = 0
+
+    class _SyncNonFunctionTool(BaseTool):
+
+      def __init__(self):
+        super().__init__(name='custom_tool', description='desc')
+        # Sync attribute so _is_sync_tool returns True.
+        self.func = lambda: 'unused'
+
+      async def run_async(self, *, args, tool_context):
+        nonlocal run_async_call_count
+        run_async_call_count += 1
+        return {'via': 'run_async'}
+
+    tool = _SyncNonFunctionTool()
+    model = testing_utils.MockModel.create(responses=[])
+    agent = Agent(name='test_agent', model=model, tools=[tool])
+    invocation_context = await testing_utils.create_invocation_context(
+        agent=agent, user_content=''
+    )
+    tool_context = ToolContext(
+        invocation_context=invocation_context,
+        function_call_id='test_id',
+    )
+
+    result = await _call_tool_in_thread_pool(tool, {}, tool_context)
+
+    assert result == {'via': 'run_async'}
+    assert run_async_call_count == 1
+
+  @pytest.mark.asyncio
+  async def test_set_model_response_tool_falls_back_to_run_async(self):
+    """SetModelResponseTool — the real-world non-FunctionTool sync tool."""
+
+    class _Schema(BaseModel):
+      answer: str
+
+    tool = SetModelResponseTool(output_schema=_Schema)
+    # Precondition: this is the code path the bug report referenced.
+    assert _is_sync_tool(tool)
+
+    model = testing_utils.MockModel.create(responses=[])
+    agent = Agent(name='test_agent', model=model, tools=[tool])
+    invocation_context = await testing_utils.create_invocation_context(
+        agent=agent, user_content=''
+    )
+    tool_context = ToolContext(
+        invocation_context=invocation_context,
+        function_call_id='test_id',
+    )
+
+    result = await _call_tool_in_thread_pool(
+        tool, {'answer': 'hello'}, tool_context
+    )
+
+    assert result == {'answer': 'hello'}
+
 
 class TestToolThreadPoolConfig:
   """Tests for the tool_thread_pool_config in RunConfig."""