Skip to content

Fix PluginName empty when using Ollama/Gemini with AddFromObject (#13516)#13672

Open
aayushbaluni wants to merge 1 commit intomicrosoft:mainfrom
aayushbaluni:fix/13516-ollama-plugin-name-empty
Open

Fix PluginName empty when using Ollama/Gemini with AddFromObject (#13516)#13672
aayushbaluni wants to merge 1 commit intomicrosoft:mainfrom
aayushbaluni:fix/13516-ollama-plugin-name-empty

Conversation

@aayushbaluni
Copy link

Description

Fixes #13516

FunctionCallContentBuilder used FunctionName.Parse(fqn) with the default separator - when building FunctionCallContent from streaming updates. However, different AI connectors use different separators:

  • Ollama/ChatClient (FullyQualifiedAIFunction): _ (e.g. time_ReadFile)
  • Gemini: _
  • OpenAI: -
  • FunctionChoiceBehavior: .

When using kernel.Plugins.AddFromObject(plugin, pluginName: "time") with Ollama, the model returns time_ReadFile but the builder parsed with -, producing pluginName=null and functionName="time_ReadFile".

Changes

  • Added ParseFullyQualifiedFunctionName helper that tries separators ., _, - when parsing
  • Added unit tests for underscore and dot separator formats

Root Cause

FunctionCallContentBuilder.Build() called FunctionName.Parse(fqn) without a separator, defaulting to -. Connectors using _ or . produced unparseable names, leaving PluginName empty.

Made with Cursor

…rosoft#13516)

FunctionCallContentBuilder.Parse used default separator '-' but Ollama
and Gemini connectors use '_' (FullyQualifiedAIFunction), and
FunctionChoiceBehavior uses '.'. Try separators '.', '_', '-' when
parsing fully-qualified function names to support all connector formats.
@aayushbaluni aayushbaluni requested a review from a team as a code owner March 17, 2026 17:33
@aayushbaluni
Copy link
Author

@microsoft-github-policy-service agree

Copy link

@chetantoshniwal chetantoshniwal left a comment

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 4 | Confidence: 86%

✗ Correctness

The new ParseFullyQualifiedFunctionName method introduces a correctness regression: it greedily tries underscore and hyphen as plugin-function separators, meaning any function name naturally containing those characters (e.g., 'get_weather' with no plugin) will be falsely split into a plugin and function name. The PluginName null-check on line 185 provides no real protection because FunctionName.Parse will always yield a non-null PluginName when the separator is found in the string. Additionally, the new method is inserted between the XML doc comments of TrackStreamingFunctionCallUpdate and the method itself, breaking the documentation association for that method.

✗ Security Reliability

The new ParseFullyQualifiedFunctionName method introduces ambiguous function name parsing by trying underscore and hyphen as plugin-function separators. Since underscores and hyphens are extremely common in function names themselves (e.g., get_weather, read_file), any such function without a plugin prefix will be incorrectly split—get_weather becomes plugin=get, function=weather. This can cause silent misrouting of function calls or resolution failures at runtime, which is both a reliability and a potential security concern (invoking the wrong function). The greedy first-match-wins approach on common characters significantly broadens the attack surface if an adversarial model response can influence function name parsing.

✗ Test Coverage

The new ParseFullyQualifiedFunctionName method supports three separators (".", "_", "-") plus a fallback path when none match, but the tests only cover dot and underscore. The hyphen separator path and the no-separator fallback path are untested. Additionally, there is no test for ambiguous underscore names (e.g., my_plugin_MyFunction) where the first underscore split may produce an unexpected plugin/function split, nor for names containing multiple separator types (e.g., time.Read_File) to verify that dot takes priority.

✗ Design Approach

The change introduces a heuristic ParseFullyQualifiedFunctionName helper that tries three separators in order (".", "_", "-") to split a fully-qualified AI function name into plugin and function parts. While this fixes the reported Ollama/Gemini underscore case, the approach is fundamentally ambiguous: the same string can be parsed differently depending on which separator appears first, and the parser has no way to distinguish a separator character that is part of a plugin or function name from an actual delimiter. For example, a function named send_email in a plugin named tools would be sent as tools_send_email; the heuristic would correctly split it, but a plugin named my_tools_v2 with function ReadFile would be sent as my_tools_v2_ReadFile and incorrectly parsed as plugin=my, function=tools_v2_ReadFile. The '-' separator is especially risky because hyphens are common in identifiers. The root cause is that different AI connectors format names differently on the way out, yet the parser on the way back in has no record of which separator was used. The correct fix is to either (1) make each connector pass its separator into the builder so it is used for parsing, or (2) resolve the ambiguity by looking up the incoming name against the kernel's registered function registry rather than guessing from the string alone.

Flagged Issues

  • Heuristic separator guessing is fundamentally ambiguous: any bare function name containing '_' or '-' (e.g., get_weather, get-weather) will be falsely split into a plugin and function name because FunctionName.Parse always produces a non-null PluginName when the separator is found. This causes silent misrouting of function calls and can invoke unintended functions. The same problem affects plugin names containing separators (e.g., my_tools_v2_ReadFile splits as plugin=my, function=tools_v2_ReadFile).
  • Critical test gaps: the hyphen separator branch, the no-separator fallback path (plain function name like ReadFile), and bare function names containing separator characters (e.g., get_weather with no plugin) are all untested. The current tests only validate the happy path where a plugin prefix genuinely exists.

Suggestions

  • Thread the separator from the connector layer into FunctionCallContentBuilder (e.g., via a constructor parameter or per-call context) so parsing is unambiguous. Alternatively, resolve function names by looking them up against the kernel's registered function registry rather than guessing from the string.
  • If multi-separator guessing must stay, add a validation step that checks the parsed plugin/function actually exists in the kernel before accepting the split; fall back to treating the whole string as a function name if not. As a quick fix, at minimum remove '-' from the candidate separators and document the known ambiguity.
  • Move the new method above the XML doc block or below TrackStreamingFunctionCallUpdate to avoid orphaning the <param> documentation for that method.
  • Add comprehensive test coverage: (a) bare function name with underscores and no plugin (e.g., get_weather), (b) hyphen separator (e.g., time-ReadFile), (c) plain function name with no separator (e.g., ReadFile), (d) name with multiple separator types (e.g., time.Read_File) to verify priority, (e) ambiguous multi-underscore name (e.g., my_plugin_MyFunction) to document the first-match behavior.

Automated review by chetantoshniwal's agents

Comment on lines +180 to +184
foreach (var separator in new[] { ".", "_", "-" })
{
if (fullyQualifiedName.Contains(separator, StringComparison.Ordinal))
{
var parsed = FunctionName.Parse(fullyQualifiedName, separator);

Choose a reason for hiding this comment

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

Bug: This separator-guessing loop will falsely split any bare function name containing ., _, or -. For example, FunctionName.Parse("get_weather", "_") produces PluginName="get", Name="weather", and the null check on line 185 passes because a non-null PluginName is always produced when the separator exists. Names with multiple underscores (e.g., my_tools_v2_ReadFile) split incorrectly as plugin=my, function=tools_v2_ReadFile. The separator must come from the connector that formatted the name, not be guessed from the string.

Suggested change
foreach (var separator in new[] { ".", "_", "-" })
{
if (fullyQualifiedName.Contains(separator, StringComparison.Ordinal))
{
var parsed = FunctionName.Parse(fullyQualifiedName, separator);
// TODO: accept the separator used by the originating connector instead of guessing.
private static FunctionName ParseFullyQualifiedFunctionName(string fullyQualifiedName)
{
return FunctionName.Parse(fullyQualifiedName);
}

if (fullyQualifiedName.Contains(separator, StringComparison.Ordinal))
{
var parsed = FunctionName.Parse(fullyQualifiedName, separator);
if (parsed.PluginName is not null)

Choose a reason for hiding this comment

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

Security: The parsed.PluginName is not null check is insufficient—any name containing the separator produces a non-null PluginName. An adversarial model response like admin_DeleteAllData would be parsed as plugin=admin, function=DeleteAllData, potentially routing to an unintended privileged function.

}
}
}

Choose a reason for hiding this comment

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

Including - as a candidate separator will silently mis-parse any hyphenated function name as a plugin-qualified call. Hyphens are far more common in identifiers than as SK plugin separators and should not be probed here.

Comment on lines +173 to +194

/// <summary>
/// Parses a fully-qualified function name into plugin and function parts.
/// Tries multiple separators (".", "_", "-") since different AI connectors use different formats.
/// </summary>
private static FunctionName ParseFullyQualifiedFunctionName(string fullyQualifiedName)
{
foreach (var separator in new[] { ".", "_", "-" })
{
if (fullyQualifiedName.Contains(separator, StringComparison.Ordinal))
{
var parsed = FunctionName.Parse(fullyQualifiedName, separator);
if (parsed.PluginName is not null)
{
return parsed;
}
}
}

return FunctionName.Parse(fullyQualifiedName);
}

Choose a reason for hiding this comment

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

Structural issue: inserting the new method here splits the XML <param> doc comments (lines 170-172) from their target method TrackStreamingFunctionCallUpdate (now at line 195), orphaning the parameter documentation. Move this method above the doc block or below TrackStreamingFunctionCallUpdate.

}

[Fact]
public void ItShouldParseUnderscoreSeparatorForOllamaAndGeminiConnectors()

Choose a reason for hiding this comment

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

The test name time_ReadFile is unambiguous because time is clearly a plugin. Add a test for a bare function name containing underscores (e.g., read_file with no plugin) to verify it is NOT incorrectly split into plugin=read, name=file.

Assert.Equal("ReadFile", functionCall.FunctionName);
}

private static StreamingChatMessageContent CreateStreamingContentWithFunctionCallUpdate(int choiceIndex, int functionCallIndex, string? callId, string? name, string? arguments, int requestIndex = 0)

Choose a reason for hiding this comment

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

Missing test coverage: (1) the hyphen separator path is untested—add a test with e.g. "time-ReadFile" asserting PluginName="time", FunctionName="ReadFile"; (2) the fallback FunctionName.Parse(fullyQualifiedName) path is untested—add a test with a plain name like "ReadFile" asserting PluginName is null and FunctionName is "ReadFile".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: The function call plugin name printed is empty

2 participants