Python: Improve function call invocation parameter consistency#14014
Conversation
b0cc638 to
d47910b
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR standardizes passing function_behavior into Kernel.invoke_function_call across multiple internal call sites, adds a warning when it’s omitted, and introduces unit tests around allowlist/filter behavior.
Changes:
- Thread
function_behaviorthrough realtime + agent invocation paths for consistent allowlist/filter validation behavior. - Add warning log in
Kernel.invoke_function_callwhenfunction_behavioris omitted. - Add unit tests covering allow/deny filtering and missing
function_behaviorscenarios.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| python/semantic_kernel/kernel.py | Adds warning when function_behavior is missing (no allowlist validation). |
| python/semantic_kernel/connectors/ai/open_ai/services/_open_ai_realtime.py | Passes execution settings’ function_choice_behavior into invoke_function_call. |
| python/semantic_kernel/agents/open_ai/responses_agent_thread_actions.py | Threads function_behavior through internal async function-call gather. |
| python/semantic_kernel/agents/open_ai/assistant_thread_actions.py | Adds function_behavior parameter and forwards it into invocation. |
| python/semantic_kernel/agents/azure_ai/agent_thread_actions.py | Adds function_behavior parameter and forwards it into invocation. |
| python/semantic_kernel/agents/bedrock/bedrock_agent.py | Forwards agent’s configured function_choice_behavior into invocation. |
| python/tests/unit/kernel/test_kernel.py | Adds tests for filter blocking/allowing and warning when behavior omitted. |
| python/tests/unit/connectors/ai/open_ai/services/test_openai_realtime.py | Adds tests ensuring realtime path forwards function_behavior and enforces filters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 92%
✗ Correctness
The PR correctly threads
function_behaviortokernel.invoke_function_callfrom the Bedrock agent and Realtime connector paths, and updates the_invoke_function_callshelper signatures in Azure AI, Assistant, and Responses agent thread actions to accept the parameter. However, the callers of those helpers (e.g.,azure_ai/agent_thread_actions.py:234-236,azure_ai/agent_thread_actions.py:1123-1124,assistant_thread_actions.py:262-264,assistant_thread_actions.py:565-567) are not updated to passfunction_behaviorthrough, so it always arrives asNonefor those agent paths. Combined with the newelif function_behavior is None:warning inkernel.py, this means every function call from the Azure AI and OpenAI Assistant agents will now emit a warning-level log message that did not exist before — producing log noise in production for legitimate agent usage without any actual security benefit.
✓ Security Reliability
This PR correctly threads function_behavior through the realtime connector and bedrock agent to enable allowlist validation for function calls, closing a real security gap. The kernel.py defensive warning is well-placed. The agent plumbing (azure_ai, assistant, responses) adds the parameter to _invoke_function_calls signatures but calers don't pass it yet, meaning those paths remain unprotected—this is not a regression and is consistent with the stated incremental approach. No blocking security or reliability issues found.
✓ Test Coverage
The PR adds function_behavior parameter plumbing to several agents and the realtime connector. Tests adequately cover the realtime connector path and kernel-level behavior (blocking, allowing, warning). However, there are notable test coverage gaps: (1) the callers of
_invoke_function_callsin Azure AI and OpenAI Assistant agents were NOT updated to passfunction_behavior, so the new parameter always defaults to None — no tests catch this incomplete threading; (2) the Bedrock agent is the only agent that actually passesfunction_behaviorat its call site, yet has no test; (3) the Responses agent's_invoke_function_callsmethod appears to be dead code (never called within the file) while the actual direct calls at lines 256 and 561 already pass function_behavior independently of this PR's changes.
✗ Design Approach
The PR improves
Kernel.invoke_function_call()and the realtime/Bedrock/Responses paths, but the assistant and Azure agent requires-action flows are still not actually threaded withfunction_behavior. In both files, the new optional parameter is added only to the local_invoke_function_calls()helper; the live callers still omit it, so those paths continue to hitKernel.invoke_function_call()withfunction_behavior=Noneand skip the allowlist validation this PR is meant to make consistent.
Suggestions
- Thread
function_behaviorfrom the assistant requires-action entry points (assistant_thread_actions.py:262-264and:565-567) and the Azure AI requires-action entry points (azure_ai/agent_thread_actions.py:234-236and:1123-1125) into_invoke_function_calls(), rather than only widening the helper signatures.
Automated review by SergeyMenshykh's agents
Thread function_behavior parameter through internal invoke_function_call callsites where it is available but was not being passed. Add defensive debug logging when parameter is not provided. Update and add unit tests for parameter handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d47910b to
180aee8
Compare
Description
This PR improves the consistency of how the function_behavior parameter is passed through internal invoke_function_call callsites.
Changes
Testing