# Inside BotSharp: Finding bugs in the \.NET AI platform's code 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\. ![1391_BotSharp/image1.png](https://import.viva64.com/docx/blog/1391_BotSharp/image1.png) ## 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](https://learn.microsoft.com/en-us/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\. ![1391_BotSharp/image2.png](https://import.viva64.com/docx/blog/1391_BotSharp/image2.png) Picture 1 — The BotSharp web interface, provided for visualizing agent\-based systems You can find more on BotSharp in the framework's [documentation](https://botsharp.readthedocs.io/en/latest/quick-start/overview.html)\. 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\]\(https://pvs\-studio\.com/en/pvs\-studio\-eap/\) now to test those\\\. For more on PVS\\\-Studio, check out the \[official website\]\(https://pvs\-studio\.com/en/pvs\-studio/try\-free/\)\\\. ### Unused parameter ```cpp public class SettingService : ISettingService { .... private readonly ILogger _logger; public SettingService(IServiceProvider services, IConfiguration config, ILogger logger) { _services = services; _config = config; } .... } ``` The PVS\-Studio warning: [V3117](https://pvs-studio.com/en/docs/warnings/v3117/)\. Constructor parameter 'logger' is not used\. [SettingService\.cs 15](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs#L15) 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 ```cpp 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](https://pvs-studio.com/en/docs/warnings/v3002/)\. The switch statement does not cover all values of the 'AgentTaskField' enum: Status\. [FileRepository\.AgentTask\.cs 234](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Infrastructure/BotSharp.Core/Repository/FileRepository/FileRepository.AgentTask.cs#L234) 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 ```cpp public async Task GetNextInstruction( ...., List dialogs) { .... dialogs = new List { new RoleDialogModel(AgentRole.User, next) { FunctionName = nameof(HFReasoner), MessageId = messageId } }; var response = await completion.GetChatCompletions(router, dialogs); .... } ``` The PVS\-Studio warning: [V3061](https://pvs-studio.com/en/docs/warnings/v3061/)\. Parameter 'dialogs' is always rewritten in method body before being used\. [HFReasoner\.cs 45](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Infrastructure/BotSharp.Core/Routing/Reasoning/HFReasoner.cs#L45) 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 ```cpp 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](https://pvs-studio.com/en/docs/warnings/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](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/tests/BotSharp.Plugin.PizzaBot/McpServerTools/MakePaymentTool.cs#L11) 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 ```cpp private async Task> OnResponsedDone(....) { .... new TokenStatsModel { .... TextInputTokens = inputTokenDetails? .TextTokens ?? 0 - inputTokenDetails? .CachedTokenDetails? .TextTokens ?? 0, .... AudioInputTokens = inputTokenDetails? .AudioTokens ?? 0 - inputTokenDetails? .CachedTokenDetails? .AudioTokens ?? 0, .... } .... } ``` The PVS\-Studio warning: [V3123](https://pvs-studio.com/en/docs/warnings/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](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Plugins/BotSharp.Plugin.OpenAI/Providers/Realtime/RealTimeCompletionProvider.cs#L542) [V3123](https://pvs-studio.com/en/docs/warnings/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](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Plugins/BotSharp.Plugin.OpenAI/Providers/Realtime/RealTimeCompletionProvider.cs#L544) 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: ```cpp (inputTokenDetails?.TextTokens ?? 0) - ( inputTokenDetails? .CachedTokenDetails? .TextTokens ?? 0) ``` ### Potential null dereference **Issue 1** ```cpp public async Task GetChatCompletions(....) { .... var toolcall = response.choices.FirstOrDefault()? .message .tool_calls.FirstOrDefault(); responseMessage = new RoleDialogModel( AgentRole.Function, JsonSerializer.Serialize(toolcall.function) ) .... } ``` The PVS\-Studio warning: [V3146](https://pvs-studio.com/en/docs/warnings/v3146/)\. Possible null dereference of 'toolcall'\. The 'FirstOrDefault' can return default null value\. [ChatCompletionProvider\.cs 62](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Plugins/BotSharp.Plugin.MetaGLM/Providers/ChatCompletionProvider.cs#L62) 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** ```cpp public async Task InputUserText(....) { .... locator = Locator(actionParams.ContextId, htmlElementContextOut); count = await locator.CountAsync(); .... } ``` The PVS\-Studio warning: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/)\. Possible null dereference\. Consider inspecting 'locator'\. [PlaywrightWebDriver\.InputUserText\.cs 46](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Plugins/BotSharp.Plugin.WebDriver/Drivers/PlaywrightDriver/PlaywrightWebDriver.InputUserText.cs#L46) The analyzer flags another potential `null` dereference, this time in `locator.CountAsync()`\. The declaration of the `Locator` method explains why: ```cpp 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** ```cpp public async Task GetRoutableAgents(List profiles) { .... if (routableAgents.Length > 0 && profiles.Count > 0) { .... } else if (profiles == null || profiles.Count == 0) { .... } .... } ``` The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/)\. The 'profiles' object was used before it was verified against null\. Check lines: 131, 137\. [RoutingService\.cs 131](https://github.com/SciSharp/BotSharp/blob/b52cbf57f05a71a59d63b7e9f5922952a75b3ba8/src/Infrastructure/BotSharp.Core/Routing/RoutingService.cs#L131) 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](https://pvs-studio.com/en/pvs-studio/try-free/)\. See you in the next articles\!