fix(flows): prevent double-execution of FunctionTools returning None in thread pool

[Koushik-Salammagari]

Link to Issue or Description of Change

Closes #5284

Description

When RunConfig.tool_thread_pool_config is enabled, _call_tool_in_thread_pool used None as a sentinel to distinguish two cases inside run_sync_tool():

1. FunctionTool ran successfully in thread pool → return the result
2. Non-FunctionTool sync tool (e.g. SetModelResponseTool) → can't run in thread pool, fall back to tool.run_async()

The problem: None is also a valid return value from any FunctionTool whose underlying function has no explicit return statement (implicit None). Any side-effect-only tool (e.g. writing to state, logging, sending a notification) falls into this category.

Before this fix:

result = await loop.run_in_executor(executor, lambda: ctx.run(run_sync_tool))
if result is not None:   # ← False when FunctionTool returns None
    return result
# Falls through to:
return await tool.run_async(args=args, tool_context=tool_context)  # ← second execution!

After this fix:

# Module-level sentinel — can never be confused with a legitimate tool return value
_SYNC_TOOL_RESULT_UNSET = object()

# Inside run_sync_tool():
#   FunctionTool path  → return tool.func(**args)   (may be None)
#   Non-FunctionTool   → return _SYNC_TOOL_RESULT_UNSET

result = await loop.run_in_executor(executor, lambda: ctx.run(run_sync_tool))
if result is not _SYNC_TOOL_RESULT_UNSET:   # ← True even when result is None
    return result

Changes

• src/google/adk/flows/llm_flows/functions.py: introduce _SYNC_TOOL_RESULT_UNSET sentinel; update run_sync_tool and the guard check.
• tests/unittests/flows/llm_flows/test_functions_thread_pool.py: consolidate regression tests into a single @pytest.mark.parametrize test covering 6 return values.

Testing Plan

• test_sync_tool_executes_exactly_once — parametrized over None (implicit), None (explicit), 0, "", {}, and False; verifies each executes exactly once and the sentinel is never confused with a valid return value.
• All 30 thread pool tests pass.
• Only triggered when RunConfig.tool_thread_pool_config is set (non-default); zero impact on the default execution path.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[adk-bot]

Response from ADK Triaging Agent

Hi @Koushik-Salammagari, thank you for your contribution!

It looks like the Contributor License Agreement (CLA) check has failed. Please sign the CLA at cla.developers.google.com so we can proceed with the review.

Thanks!

[Koushik-Salammagari]

@googlebot I signed it!

[Koushik-Salammagari]

@googlebot I signed it!

[Koushik-Salammagari]

@googlebot I signed it!

[ruidazeng]

The two tests are nearly identical. Can we collapse them into one test or use @pytest.mark.parametrize? I don't have a strong opinion on this.

[Koushik-Salammagari]

Done — collapsed both tests into a single @pytest.mark.parametrize test covering 6 cases: implicit None, explicit None, 0, "", {}, and False. All 30 tests pass.

[ruidazeng]

Consider adding a falsy-but-not-None test. The bug was specifically about None. A test like this would prove the sentinel doesn't break tools returning 0, "", {}, or False.

[Koushik-Salammagari]

Added falsy-but-not-None cases (0, "", {}, False) as part of the parametrized test. These confirm the sentinel is identity-based (is not _SYNC_TOOL_RESULT_UNSET) and correctly treats all falsy return values as valid results without triggering a second execution.