C# has a reputation as a language for enterprise development but not for machine learning. BotSharp pushes back on that idea with a powerful AI platform built on .NET. We'll walk through the project and the bugs we found in its source code.
Foreword
Python has long dominated machine learning, while C# made its name in enterprise systems, web development, and game development. That's starting to shift. Tools like ML.NET and Microsoft Agent Framework show that .NET can hold its own in AI development too. BotSharp is one of these tools, and it's what this article is about. Mostly, though, we'll be digging into the bugs we found in its source code using the PVS-Studio static analyzer. Let's get started.
A word about the project
BotSharp is an open source framework written in C#. It's built for creating single-agent and multi-agent systems, where one or more AI models drive the decision-making. These models can process and generate multimedia data, and they can act by calling the tools they're given.
So how does BotSharp differ from Semantic Kernel? Both currently solve similar problems, offering components for building LLM-powered agent systems. What sets BotSharp apart is a higher level of abstraction and built-in infrastructure for managing agents and dialogs, which makes it faster to deploy typical agent-based solutions.
Picture 1 — The BotSharp web interface, provided for visualizing agent-based systems
You can find more on BotSharp in the framework's documentation. For now, though, let's get to the interesting part, the bugs we found in its code.
Error review
PVS-Studio found the bugs covered below in the project's version 5.2, the latest one available as of this writing.
PVS-Studio is a static analysis tool for C#, C, C++, and Java code, built to catch bugs and potential vulnerabilities. Support for Go, JavaScript, and TypeScript is on the way too, and you can sign up now to test those. For more on PVS-Studio, check out the official website.
Unused parameter
public class SettingService : ISettingService
{
....
private readonly ILogger _logger;
public SettingService(IServiceProvider services,
IConfiguration config,
ILogger<SettingService> logger)
{
_services = services;
_config = config;
}
....
}
The PVS-Studio warning: V3117. Constructor parameter 'logger' is not used. SettingService.cs 15
Just a sloppy typo: the logger parameter gets passed into the constructor, but nobody assigns it to the matching _logger field. Nothing breaks so far, simply because that the _logger field isn't used anywhere yet. But it's still a trap waiting to be triggered.
Sooner or later someone may want to add logging. They'll spot the field already sitting there, write the usual logging call, and the code will compile without any complaints. Then the first call in prod throws a NullReferenceException out of nowhere. Landmines like this are exactly the kind of thing you don't want to find later.
Missing case in a switch statement
public async Task UpdateAgentTask(....)
{
....
switch (field)
{
case AgentTaskField.Name:
metaData.Name = task.Name;
break;
case AgentTaskField.Description:
metaData.Description = task.Description;
break;
case AgentTaskField.Enabled:
metaData.Enabled = task.Enabled;
break;
case AgentTaskField.Content:
content = task.Content;
break;
case AgentTaskField.All:
metaData.Name = task.Name;
metaData.Description = task.Description;
metaData.Enabled = task.Enabled;
content = task.Content;
break;
}
....
}
The PVS-Studio warning: V3002. The switch statement does not cover all values of the 'AgentTaskField' enum: Status. FileRepository.AgentTask.cs 234
This switch statement updates the metaData properties based on the flag it receives. Nearly every value in the AgentTaskField enum shows up here, only Status got left out.
Because of this gap, the app misbehaves in two different scenarios. If there's an explicit request to update Status, the switch just ignores it and metaData stays unchanged. The case AgentTaskField.All block has a problem too: it's supposed to overwrite every metadata property with task properties, but since Status got left out, that block doesn't fully do what it's supposed to.
It's a good illustration of how extending an enum without checking every switch statement that depends on it can quietly introduce hidden bugs.
Losing a parameter's value
public async Task<FunctionCallFromLlm> GetNextInstruction(
....,
List<RoleDialogModel> dialogs)
{
....
dialogs = new List<RoleDialogModel>
{
new RoleDialogModel(AgentRole.User, next)
{
FunctionName = nameof(HFReasoner),
MessageId = messageId
}
};
var response = await completion.GetChatCompletions(router, dialogs);
....
}
The PVS-Studio warning: V3061. Parameter 'dialogs' is always rewritten in method body before being used. HFReasoner.cs 45
The GetNextInstruction method takes a dialogs collection as a parameter, but instead of adding to it, the method completely replaces the collection with a new, empty template.
The parameter most likely stories some conversation context needed to generate a relevant response. Overwriting the variable wipes that context clean right before the request goes to GetChatCompletions, which can noticeably hurt the quality of the response.
The fix is straightforward. Add the new data with Add or, if necessary, leave the dialogs collection untouched, or build a new collection based on dialogs with the extra element added in.
A copy-pasted condition
public string Make_Payment(
[Description("order number"), Required] string order_number,
[Description("total amount"), Required] int total_amount)
{
if (order_number is null)
{
throw new McpException("Missing required argument 'order_number'");
}
if (order_number is null)
{
throw new McpException("Missing required argument 'total_amount'");
}
}
The PVS-Studio warning: V3021. There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless MakePaymentTool.cs 11
This one comes down to careless copying. The method is meant to check two required parameters, but since both if conditions are identical, order_number gets validated twice, while the second argument never gets checked at all.
The exception text in the second block gives away the original intent, it was supposed to check total_amount. Bugs like this slip right past a quick read, but static analysis catches them immediately.
A misunderstanding of operator precedence
private async Task<List<RoleDialogModel>> OnResponsedDone(....)
{
....
new TokenStatsModel
{
....
TextInputTokens = inputTokenDetails?
.TextTokens ?? 0 - inputTokenDetails?
.CachedTokenDetails?
.TextTokens ?? 0,
....
AudioInputTokens = inputTokenDetails?
.AudioTokens ?? 0 - inputTokenDetails?
.CachedTokenDetails?
.AudioTokens ?? 0,
....
}
....
}
The PVS-Studio warning:
V3123. Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. RealTimeCompletionProvider.cs 542
V3123. Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. RealTimeCompletionProvider.cs 544
This code reads as perfectly logical to a human, but the compiler sees it differently.
The developer attempted to calculate a difference, with each operand guarded against null by the ?? operator. But the subtraction operator - has a higher precedence than the ?? null-coalescing operator, so instead of the intended (A ?? 0) - (B ?? 0) what actually gets evaluated is A ?? ((0 - B) ?? 0).
To get the intended behavior, the expressions around ?? need to be explicitly wrapped in parentheses:
(inputTokenDetails?.TextTokens ?? 0) - ( inputTokenDetails?
.CachedTokenDetails?
.TextTokens ?? 0)
Potential null dereference
Issue 1
public async Task<RoleDialogModel> GetChatCompletions(....)
{
....
var toolcall = response.choices.FirstOrDefault()?
.message
.tool_calls.FirstOrDefault();
responseMessage = new RoleDialogModel(
AgentRole.Function, JsonSerializer.Serialize(toolcall.function)
)
....
}
The PVS-Studio warning: V3146. Possible null dereference of 'toolcall'. The 'FirstOrDefault' can return default null value. ChatCompletionProvider.cs 62
Here, the toolcall variable gets its value from a call chain that uses the FirstOrDefault method twice.
The developer took every precaution in that chain, so if either collection turns out empty, toolcall ends up as null without throwing anything.
The problem shows up right after, inside JsonSerializer.Serialize, where the variable gets dereferenced with no check beforehand. Sooner or later that throws a NullReferenceException.
Issue 2
public async Task<BrowserActionResult> InputUserText(....)
{
....
locator = Locator(actionParams.ContextId, htmlElementContextOut);
count = await locator.CountAsync();
....
}
The PVS-Studio warning: V3080. Possible null dereference. Consider inspecting 'locator'. PlaywrightWebDriver.InputUserText.cs 46
The analyzer flags another potential null dereference, this time in locator.CountAsync().
The declaration of the Locator method explains why:
private ILocator? Locator(....)
{
....
else if (!string.IsNullOrEmpty(context.ElementName))
{
....
if (count == 0)
{
_logger.LogError(....);
return null;
}
else if (count > 1)
{
_logger.LogError(....);
return null;
}
}
else
{
if (context.Index < 0)
{
_logger.LogError(....);
return null;
}
....
}
return element;
}
There are three separate paths where this method returns null. Yet in the code the analyzer points to, that return value never gets checked.
Issue 3
public async Task<RoutableAgent[]> GetRoutableAgents(List<string> profiles)
{
....
if (routableAgents.Length > 0 && profiles.Count > 0)
{
....
}
else if (profiles == null || profiles.Count == 0)
{
....
}
....
}
The PVS-Studio warning: V3095. The 'profiles' object was used before it was verified against null. Check lines: 131, 137. RoutingService.cs 131
This one comes down to a validation order problem. The developer planned for profiles possibly being uninitialized and added a check for it in the else if block. But that check comes too late, the if statement above it already calls profiles.Count. Swapping the two blocks fixes the logic.
Conclusion
I hope this tour through the BotSharp source code was useful, and hopefully a bit entertaining too. Defects like these, an unclear operator precedence, a forgotten case in a switch statement, a lost argument value, blend right into large codebases. Finding them by hand in a massive project is a lot harder than it sounds, and that's exactly where static analysis earns its keep.
If this article has you wanting to check your own project, you can try the analyzer free.
See you in the next articles!
Top comments (0)