Skip to content

Fix FunctionInvokingChatClient to respect ChatOptions.Tools modifications by function tools #7218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
Copilot wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Fix FunctionInvokingChatClient to respect ChatOptions.Tools modifications by function tools #7218

Copilot wants to merge 4 commits into from
+768 −49

Conversation

Copy link
Contributor

Copilot AI commented Jan 20, 2026 •
edited
Loading

  • Understand the issue: FunctionInvokingChatClient caches toolMap at the start of the function loop, but functions can modify ChatOptions.Tools during invocation
  • Initial fix: Added NeedsToolMapRefresh method to detect when tools have changed
  • Reviewer feedback: @stephentoub suggested removing the map entirely and using linear search instead
  • Implement simplified approach:
    • Removed CreateToolsMap and NeedsToolMapRefresh methods
    • Added FindTool helper for linear search through tool lists
    • Added AnyToolsRequireApproval helper to check if any tool requires approval
    • Added HasAnyTools helper to check if there are any tools
    • Updated all callers to use direct tool list searches instead of map lookups
  • Run tests to validate changes (all 66 FunctionInvokingChatClient tests pass)
Original prompt

This section details on the original issue you should resolve

<issue_title>FunctionInvokingChatClient should respect ChatOptions.Tools modifications by function tool</issue_title>
<issue_description>FunctionInvokingChatClient provides a FunctionInvocationContext that contains ChatOptions when invoking function tools.

If one of the executed functions modifies the Tools list on ChatOptions, this updated set of tools will be advertised to the downstream ChatClient. However, FunctionInvokingChatClient does not respect these updates when invoking subsequent functions, which can cause failures. FunctionInvokingChatClient currently caches a dictionary of tool name to tool, which is calculated at the start of the function loop, and before any modifications might happen.

Here is a step by step workflow explaining how a failure may happen:

  1. FICC is called, and it calls the downstream ChatClient.
  2. FICC gets a FCC back and FICC calls the appropriate Function tool (lets say Function A)
  3. Function A modifies ChatOptions.Tools on FunctionInvocationContext by adding Function B to the list.
  4. FICC calls the downstream ChatClient.
  5. FICC gets a FCC back for Function B.
  6. FICC fails to invoke Function B, saying that it cannot find Function B

Also see microsoft/agent-framework#3083</issue_description>

<agent_instructions>Consider fixing this by refreshing the tool map before processing functions if it's not the initial set of invocations (in which case the tool map will by definition be up to date) or if the current tools have become inconsistent with the tool map. I'm hoping this is as simple as an O(N) traversal of the current tools validating that they're all there and that there aren't other tools in the map that would indicate tools were removed from ChatOptions.Tools. Please make sure this is very well tested, including cases where tools are added, removed, reordered, etc. during function invocation, both when there are AdditionalTools and not. We want to make this as efficient as possible, so we don't want to recompute the tools map unnecessarily, but we also need it to be functionally correct, so the tools map must always be accurate when querying it.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Microsoft Reviewers: Open in CodeFlow

Copilot AI and others added 2 commits January 20, 2026 13:35
@stephentoub
Fix FunctionInvokingChatClient to respect ChatOptions.Tools modificat…
8d82e16 
…ions by function tools

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub
Address code review feedback: improve comments and test assertions
dc4158c 
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix FunctionInvokingChatClient to respect ChatOptions.Tools modifications Fix FunctionInvokingChatClient to respect ChatOptions.Tools modifications by function tools Jan 20, 2026
Copilot AI requested a review from stephentoub January 20, 2026 13:43
@stephentoub
Copy link
Member

stephentoub commented Jan 21, 2026

@copilot, would it make more sense / be cleaner to just get rid of the map entirely?

I believe we added it originally out of concern for the case where there are lots of tools and lots of function requests, but it's rare to have more than a few function requests in a single response, if we consider the number of tools to be a constant controlled by the client then we're still O(N) in the number of tool requests, and having to keep the map up to date adds even more overhead that the 90% case is having to pay.

@stephentoub
Remove tool map in favor of linear search through tool lists
fc8c227 
Per reviewer feedback, removed the Dictionary-based tool map entirely
and replaced it with linear search through AdditionalTools and
options.Tools. This simplifies the code and removes the overhead of
maintaining/refreshing the map, which is acceptable since:
- Most function invocations have only a few function calls
- Tool lists are typically small
- Linear search avoids dictionary allocation and synchronization overhead

Added helper methods:
- FindTool: Linear search for a tool by name
- AnyToolsRequireApproval: Checks if any tool requires approval
- HasAnyTools: Checks if there are any tools in the lists

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 21, 2026

@copilot, would it make more sense / be cleaner to just get rid of the map entirely?

I believe we added it originally out of concern for the case where there are lots of tools and lots of function ...

Good point. Removed the map entirely in fc8c227. Now using linear search through AdditionalTools and options.Tools via a FindTool helper method. This simplifies the code and naturally handles tool modifications since we search the current lists on each lookup.

stephentoub
stephentoub reviewed Jan 21, 2026
View reviewed changes
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
int consecutiveErrorCount = 0;

(Dictionary<string, AITool>? toolMap, bool anyToolsRequireApproval) = CreateToolsMap(AdditionalTools, options?.Tools); // all available tools, indexed by name
bool anyToolsRequireApproval = AnyToolsRequireApproval(AdditionalTools, options?.Tools);
Copy link
Member

@stephentoub stephentoub Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, do we need this here? What's consuming this value before it's recomputed in the for loop below?

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
int consecutiveErrorCount = 0;

(Dictionary<string, AITool>? toolMap, bool anyToolsRequireApproval) = CreateToolsMap(AdditionalTools, options?.Tools); // all available tools, indexed by name
bool anyToolsRequireApproval = AnyToolsRequireApproval(AdditionalTools, options?.Tools);
Copy link
Member

@stephentoub stephentoub Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, same question

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
private static AITool? FindTool(string name, params ReadOnlySpan<IList<AITool>?> toolLists)
{
// Search in reverse order so that tools from later lists (e.g., options.Tools)
// take precedence over tools from earlier lists (e.g., AdditionalTools).
Copy link
Member

@stephentoub stephentoub Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, would it be simpler to just change the contract so that tools from earlier lists take precedence over tools from later lists? Then this could just be a foreach. We'd just need to fix up call sites, assuming it's possible and easy.

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
{
for (int j = 0; j < count; j++)
{
if (string.Equals(toolList[j].Name, name, StringComparison.Ordinal))
Copy link
Member

@stephentoub stephentoub Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, this condition should also include whether the tool is an AIFunctionDeclaration, right?

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
anyApprovalRequired = true;
break;
}
anyApprovalRequired = true;
Copy link
Member

@stephentoub stephentoub Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, this could be simplified to:

 anyApprovalRequired |= FindTool(functionCall.Name, toolLists)?.GetService<ApprovalRequiredAIFunction>() is not null;

test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task RespectsChatOptionsToolsModificationsByFunctionTool_AddTool(bool streaming)
Copy link
Member

@stephentoub stephentoub Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, please ensure there are also tests validating when the added/removed/changed functions require approval.

Copilot AI requested a review from stephentoub January 21, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephentoub stephentoub Awaiting requested review from stephentoub

At least 1 approving review is required to merge this pull request.

Assignees

@stephentoub stephentoub

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

FunctionInvokingChatClient should respect ChatOptions.Tools modifications by function tool

2 participants

@stephentoub