Python: always_require tool approval silently bypassed when preceded by a declaration-only or unknown call in the same batch

[CorgiBoyG]

Description

In the Python core, a tool configured with approval_mode="always_require" can be silently bypassed (executed or returned as user input without surfacing an approval request) when another call precedes it in the same model tool-call batch. The approval outcome depends on the position of the call within the batch rather than on the call itself.

Root cause

_try_execute_function_call_groups in python/packages/core/agent_framework/_tools.py classifies the batch with a loop that stops at the first matching call:

for function_call in actionable_calls:
    function_name = function_call.name
    if function_name in approval_tool_names:
        requires_approval = True
        break
    if function_name in declaration_only_tool_names or function_name in additional_tool_names:
        has_declaration_only_call = True
        break
    if config.get("terminate_on_unknown_calls", False) and function_name not in tool_map:
        raise KeyError(f'Error: Requested function "{function_name}" not found.')

Because each branch breaks (or raises) at the first hit, classification travels with the call's position, not with the call:

• A declaration-only call appearing before an always_require call makes the whole batch classified as declaration-only user input, so the approval gate is never surfaced.
• With terminate_on_unknown_calls=True, an unknown call appearing before an always_require call raises KeyError first, so the higher-priority approval pause is never reached.

Reproduction

Model returns one batch with two calls where a declaration-only (or unknown) call is ordered first and an always_require tool is second. Expected: an approval request is surfaced for the always_require tool. Actual: no approval request; the batch is returned as user input (declaration-first) or terminated with KeyError (unknown-first).

Expected invariant

Approval enforcement must be independent of call order within a batch. Classification should scan the whole batch and apply a fixed priority: approval pause > declaration-only user-input > unknown-call termination.

Scope

python/packages/core/agent_framework/_tools.py batch classification in _try_execute_function_call_groups. Verified present on current main.

Note: this is distinct from #6385 (which concerned the opposite direction, never_require tools being over-approved) and from #7881 (a concurrency-groups feature that retains the current order-dependent classification).

I'm working on a fix with regression tests and will open a PR shortly with Fixes #NNNN.

[he-yufeng]

Confirmed on current main, and there is a nice smoking gun: the comment right above the loop says "A user-input pause takes precedence over unknown-call termination in mixed batches", but the break-on-first-hit loop cannot honor that invariant unless the pause-worthy call happens to come first. All three orderings are broken the way you describe: declaration-only before always_require masks the approval, unknown before always_require (or before declaration-only) raises KeyError before the pause is ever considered.

Fix I would ship: scan the whole batch with explicit precedence instead of first-hit-wins. Collect requires_approval / has_declaration_only_call across all actionable calls (approval short-circuits only upward), and evaluate terminate_on_unknown_calls only after, when neither pause classification fired. That makes the code match its own comment, and single-classification batches behave exactly as before. Tests: the two masked orderings from your report, the unknown+terminate mixed batch, and the pure unknown+terminate case still raising. Happy to open the issue-linked draft.

[he-yufeng]

One process note before I open anything: the function-calling-loop spec (docs/specs/004) asks external contributors to check with the core team before implementing in this area, so I am holding the draft until someone from the team confirms the intended precedence and the affected scenario-matrix rows.

The fix and regression tests are ready locally and follow the shape above: whole-batch classification with explicit precedence (approval > declaration-only pause > unknown-call termination), unknowns fatal only when neither pause fired. Tests pin the two masked orderings, the unknown+terminate mixed batch, and that a lone unknown call still raises. Happy to open the issue-linked draft the moment the team gives direction.

[CorgiBoyG]

Thanks for calling out the ownership guidance. I had already opened #8080 with the reproduction and regression coverage, but the core team should confirm the intended precedence, affected scenario-matrix rows, and validation scope before this moves forward. I've converted the PR to draft and will hold it there. Happy to adjust or step back based on the team's direction; this should also avoid duplicate implementations.