Add message queue and rule actions

[iceljc]

PR Type

Enhancement, Tests

---

Description

• Refactored rule system with separate criteria and action execution

• Added message queue abstraction with RabbitMQ implementation

• Implemented four rule action types: chat, HTTP, function call, message queue

• Added rule trigger hooks for pre/post execution lifecycle events

• Created code script criteria provider for rule validation

• Added utility methods for safe dictionary value extraction with JSON support

---

Diagram Walkthrough

flowchart LR
  A["Rule Trigger"] --> B["Rule Criteria Validation"]
  B --> C["Rule Action Execution"]
  C --> D["Chat Action"]
  C --> E["HTTP Action"]
  C --> F["Function Call Action"]
  C --> G["Message Queue Action"]
  G --> H["RabbitMQ Service"]
  H --> I["Channel Pool"]
  H --> J["Consumer Registration"]

Loading

File Walkthrough

	Relevant files
Enhancement	39 files AgentRule.cs Restructured rule model with criteria and actions                +56/-2    IMQConsumer.cs Abstract interface for message queue consumers                      +21/-0    IMQService.cs Message queue service interface with publish/subscribe      +31/-0    MQConsumerBase.cs Base class for MQ consumer implementations                              +51/-0    MQMessage.cs Generic message wrapper with metadata                                        +14/-0    MQPublishOptions.cs Publishing options for message queue operations                    +33/-0    IRuleTriggerHook.cs Hook interface for rule execution lifecycle                            +13/-0    IRuleAction.cs Rule action interface with execution contract                        +25/-1    IRuleCriteria.cs Rule criteria validation interface                                              +6/-0      RuleActionContext.cs Context data for rule action execution                                      +10/-0    RuleActionResult.cs Result model for rule action execution                                      +46/-0    RuleCriteriaContext.cs Context data for rule criteria validation                                +10/-0    RuleCriteriaResult.cs Result model for rule criteria validation                                +19/-0    RuleCriteriaOptions.cs Configuration options for criteria execution                          +39/-0    RuleTriggerOptions.cs Simplified trigger options with agent filter                          +5/-14    ObjectExtensions.cs Added safe dictionary value extraction methods                      +38/-0    RuleController.cs API endpoint for triggering rule actions                                  +42/-0    RuleEngine.cs Refactored engine with criteria and action separation        +161/-117 RuleMessagePayload.cs Payload model for rule-triggered messages                                +11/-0    RuleTriggerActionRequest.cs Request model for rule action API                                                +18/-0    ChatRuleAction.cs Chat action implementation for rule engine                              +72/-0    FunctionCallRuleAction.cs Function call action implementation                                            +44/-0    HttpRuleAction.cs HTTP request action with URL and header support                    +194/-0  MessageQueueRuleAction.cs Message queue publishing action implementation                      +128/-0  CodeScriptRuleCriteria.cs Code script-based criteria validation provider                      +130/-0  RulesPlugin.cs Plugin registration for rule actions and criteria                +9/-0      MessagingPlugin.cs Plugin for message queue configuration                                      +18/-0    AgentController.Rule.cs API endpoints for criteria providers and actions                  +9/-4      AgentRuleMongoElement.cs Updated MongoDB models for new rule structure                        +87/-3    MongoRepository.Conversation.cs Optimized state filtering with combined conditions              +1/-2      RabbitMQChannelPool.cs Channel pooling for RabbitMQ connections                                  +73/-0    RabbitMQChannelPoolFactory.cs Factory for managing RabbitMQ channel pools                            +13/-0    RabbitMQConnection.cs RabbitMQ connection management with retry logic                    +154/-0  RabbitMQController.cs API endpoints for message queue operations                              +89/-0    IRabbitMQConnection.cs Interface for RabbitMQ connection management                          +11/-0    PublishDelayedMessageRequest.cs Request/response models for message publishing                      +31/-0    ScheduledMessagePayload.cs Payload model for scheduled messages                                          +9/-0      UnsubscribeConsumerRequest.cs Request model for consumer unsubscription                                +6/-0      RabbitMQService.cs RabbitMQ service implementation with publish/subscribe      +318/-0
Configuration changes	10 files MessageQueueSettings.cs Configuration settings for message queue                                  +7/-0      RuleConstant.cs Constants for default criteria and action names                    +7/-0      Using.cs Global using statements for rules infrastructure                  +23/-1    RabbitMQConsumerConfig.cs Configuration for RabbitMQ consumer setup                                +29/-0    RabbitMQSettings.cs Configuration settings for RabbitMQ connection                      +10/-0    Using.cs Global using statements for RabbitMQ plugin                            +38/-0    BotSharp.Plugin.RabbitMQ.csproj New RabbitMQ plugin project file                                                  +22/-0    BotSharp.sln Added RabbitMQ plugin to solution                                                +11/-0    WebStarter.csproj Added RabbitMQ plugin project reference                                    +1/-0      appsettings.json Added message queue and RabbitMQ configuration                      +18/-1
Bug fix	1 files GraphDb.cs Fixed error message to use provider name                                  +1/-1
Tests	2 files DummyMessageConsumer.cs Dummy consumer for testing message handling                            +24/-0    ScheduledMessageConsumer.cs Consumer for scheduled message processing                                +25/-0
Dependencies	1 files Directory.Packages.props Added RabbitMQ.Client package dependency                                  +1/-0
Additional files	1 files RabbitMQPlugin.cs +56/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	SSRF via HTTP action Description: The BotSharp-http rule action builds a request URL (http_url) and headers (http_headers) directly from context.States (ultimately user/agent-controlled), enabling SSRF (e.g., calling http://169.254.169.254/ or internal services) and potential header injection unless explicit allowlisting/validation is added. HttpRuleAction.cs [94-177] Referred Code private string BuildUrl(RuleActionContext context) { var url = context.States.TryGetValueOrDefault<string>("http_url"); if (string.IsNullOrEmpty(url)) { throw new ArgumentNullException("Unable to find http_url in context"); } // Fill in placeholders in url var strValues = context.States.Where(x => x.Value != null && x.Value is string); foreach (var item in strValues) { var value = item.Value as string; if (string.IsNullOrEmpty(value)) { continue; } url.Replace($"{{{item.Key}}}", value); } ... (clipped 63 lines)
	Code execution pathway Description: RuleCriteria.ValidateAsync executes an agent-provided code script via ICodeProcessor.Run using inputs from RuleTriggerOptions.Criteria (CodeProcessor, CodeScriptName, ArgumentContent), which can enable remote code execution if an authenticated caller can influence these options and/or agent scripts are not strictly trusted and access-controlled. RuleCriteria.cs [23-116] Referred Code public async Task<bool> ValidateAsync(Agent agent, IRuleTrigger trigger, CriteriaExecuteOptions options) { if (string.IsNullOrWhiteSpace(agent?.Id)) { return false; } var provider = options.CodeProcessor ?? BuiltInCodeProcessor.PyInterpreter; var processor = _services.GetServices<ICodeProcessor>().FirstOrDefault(x => x.Provider.IsEqualTo(provider)); if (processor == null) { _logger.LogWarning($"Unable to find code processor: {provider}."); return false; } var agentService = _services.GetRequiredService<IAgentService>(); var scriptName = options.CodeScriptName ?? $"{trigger.Name}_rule.py"; var codeScript = await agentService.GetAgentCodeScript(agent.Id, scriptName, scriptType: AgentCodeScriptType.Src); var msg = $"rule trigger ({trigger.Name}) code script ({scriptName}) in agent ({agent.Name}) => args: {options.ArgumentContent?.RootElement.GetRawText()}."; ... (clipped 73 lines)
	Default MQ credentials Description: Default RabbitMQ credentials (UserName: guest, Password: guest) are included in configuration and, if MessageQueue.Enabled is switched on in any environment, could lead to unauthorized broker access if deployed without overriding secrets. appsettings.json [1023-1034] Referred Code "MessageQueue": { "Enabled": false, "Provider": "RabbitMQ" }, "RabbitMQ": { "HostName": "localhost", "Port": 5672, "UserName": "guest", "Password": "guest", "VirtualHost": "/" },
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🔴	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Non-descriptive identifier: The public field SelfId is non-descriptive and not self-documenting (and is a mutable field rather than a property), reducing code clarity. Referred Code public class RuleTriggerHookBase : IRuleTriggerHook { public string SelfId = string.Empty; public Task BeforeRuleActionExecuted(Agent agent, IRuleTrigger trigger, RuleActionContext context) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Broken URL templating: BuildUrl attempts placeholder replacement via url.Replace(...) but does not assign the result back to url, so placeholders are never actually replaced and can cause incorrect requests at runtime. Referred Code // Fill in placeholders in url var strValues = context.States.Where(x => x.Value != null && x.Value is string); foreach (var item in strValues) { var value = item.Value as string; if (string.IsNullOrEmpty(value)) { continue; } url.Replace($"{{{item.Key}}}", value); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Leaks internal errors: The controller returns ex.Message in HTTP 500 responses, potentially exposing internal implementation details to API callers. Referred Code catch (Exception ex) { _logger.LogError(ex, "Failed to publish scheduled message"); return StatusCode(StatusCodes.Status500InternalServerError, new PublishMessageResponse { Success = false, Error = ex.Message }); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logs message payload: The consumer logs full message data content, which can include sensitive user-provided information and is not safe for production logs. Referred Code var data = string.Empty; var config = registration.Consumer.Config as RabbitMQConsumerConfig ?? new(); try { data = Encoding.UTF8.GetString(eventArgs.Body.Span); _logger.LogInformation($"Message received on '{config.QueueName}', id: {eventArgs.BasicProperties?.MessageId}, data: {data}"); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: SSRF risk: The HTTP rule action accepts http_url from context.States and sends requests without allowlisting/validation, enabling SSRF and unintended access to internal resources. Referred Code private string BuildUrl(RuleActionContext context) { Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit context: The new authenticated rule execution endpoint does not record an audit-style log entry with actor/user identity, inputs, and outcome, making it hard to reconstruct who triggered which rule action. Referred Code [HttpPost("/rule/trigger/run")] public async Task<IActionResult> RunAction([FromBody] RuleTriggerActionRequest request) { if (request == null) { return BadRequest(new { Success = false, Error = "Request cannnot be empty." }); } var trigger = _services.GetServices<IRuleTrigger>().FirstOrDefault(x => x.Name.IsEqualTo(request.TriggerName)); if (trigger == null) { return BadRequest(new { Success = false, Error = "Unable to find rule trigger." }); } var result = await _ruleEngine.Triggered(trigger, request.Text, request.States, request.Options); return Ok(new { Success = true }); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve URL replacements Assign the result of url.Replace back to the url variable, as strings are immutable and the method returns a new string with the replacements. src/Infrastructure/BotSharp.Core.Rules/Services/HttpRuleAction.cs [102-112] // Fill in placeholders in url var strValues = context.States.Where(x => x.Value != null && x.Value is string); foreach (var item in strValues) { var value = item.Value as string; if (string.IsNullOrEmpty(value)) { continue; } - url.Replace($"{{{item.Key}}}", value); + url = url.Replace($"{{{item.Key}}}", value); } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug where string immutability is overlooked, causing the url.Replace method to have no effect, and provides the correct fix.	High
General	Return raw JSON body In GetHttpRequestBody, return the body string directly instead of re-serializing it with JsonSerializer.Serialize to avoid corrupting an already-formatted JSON string. src/Infrastructure/BotSharp.Core.Rules/Services/HttpRuleAction.cs [180-189] private string? GetHttpRequestBody(RuleActionContext context) { var body = context.States.TryGetValueOrDefault<string>("http_request_body"); if (string.IsNullOrEmpty(body)) { return null; } - return JsonSerializer.Serialize(body, BotSharpOptions.defaultJsonOptions); + return body; } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that re-serializing an already-JSON string will corrupt it, which is a significant bug, and the proposed fix is correct.	Medium
High-level	Refactor RabbitMQ connection and channel management Simplify the RabbitMQ implementation by removing the custom channel pool. Instead, use a single singleton connection to create short-lived channels on demand, which is a more standard and robust approach. Examples: src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs [198-252] var channelPool = RabbitMQChannelPoolFactory.GetChannelPool(_services, _mqConnection); var channel = channelPool.Get(); try { var args = new Dictionary<string, object?> { ["x-delayed-type"] = "direct" }; ... (clipped 45 lines) src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQChannelPool.cs [1-73] using Microsoft.Extensions.ObjectPool; using RabbitMQ.Client; namespace BotSharp.Plugin.RabbitMQ.Connections; public class RabbitMQChannelPool { private readonly ObjectPool<IChannel> _pool; private readonly ILogger<RabbitMQChannelPool> _logger; private readonly int _tryLimit = 3; ... (clipped 63 lines) Solution Walkthrough: Before: // src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs public async Task<bool> PublishAsync<T>(T payload, MQPublishOptions options) { // ... await policy.Execute(async () => { var channelPool = RabbitMQChannelPoolFactory.GetChannelPool(_services, _mqConnection); var channel = channelPool.Get(); try { // ... publish message using channel } finally { channelPool.Return(channel); } }); // ... } After: // src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs // Remove RabbitMQChannelPool.cs and RabbitMQChannelPoolFactory.cs public async Task<bool> PublishAsync<T>(T payload, MQPublishOptions options) { // ... await policy.Execute(async () => { // Create a short-lived channel directly from the singleton connection using var channel = await _mqConnection.CreateChannelAsync(); // ... publish message using channel // Channel is automatically disposed }); // ... } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies unnecessary complexity in the custom channel pooling and a potential deadlock issue, recommending a simpler, standard practice for RabbitMQ channel management.	Medium
Learned best practice	Copy mutable state dictionaries Avoid sharing the same Dictionary instance across the rule execution and the queued payload by copying context.States into a new dictionary before publishing. src/Infrastructure/BotSharp.Core.Rules/Services/MessageQueueRuleAction.cs [39-47] var payload = new RuleMessagePayload { AgentId = agent.Id, TriggerName = trigger.Name, Channel = trigger.Channel, Text = context.Text, Timestamp = DateTime.UtcNow, - States = context.States + States = context.States is null ? new() : new Dictionary<string, object?>(context.States) }; Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - When transferring metadata or other mutable collections between objects, copy the dictionary instead of assigning the same reference to avoid shared-state side effects.	Low
	Avoid blocking on async operations Avoid blocking on the async channel creation inside the pool policy; use the synchronous channel creation API or restructure to create channels asynchronously outside the pool. src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQChannelPool.cs [64-67] public IChannel Create() { - return _connection.CreateChannelAsync().ConfigureAwait(false).GetAwaiter().GetResult(); + return _connection.CreateChannel(); } Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - In async/concurrent code, avoid deadlocks by not blocking on async calls (e.g., using GetAwaiter().GetResult()).	Low
More

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	SSRF via HTTP action Description: The HTTP rule action builds fullUrl and sends an outbound request using values from context.States (e.g., http_url, query params, headers), enabling SSRF against internal services (e.g., http://169.254.169.254/, http://localhost/admin) and potential header abuse unless strict allowlisting and URL validation are enforced. HttpRuleAction.cs [43-67] Referred Code // Build the full URL var fullUrl = BuildUrl(context); using var client = _httpClientFactory.CreateClient(); // Add headers AddHttpHeaders(client, context); // Create request var request = new HttpRequestMessage(httpMethod, fullUrl); // Add request body if provided var requestBodyStr = GetHttpRequestBody(context); if (!string.IsNullOrEmpty(requestBodyStr)) { request.Content = new StringContent(requestBodyStr, Encoding.UTF8, MediaTypeNames.Application.Json); } _logger.LogInformation("Executing HTTP rule action for agent {AgentId}, URL: {Url}, Method: {Method}", agent.Id, fullUrl, httpMethod); ... (clipped 4 lines)
	Code execution in criteria Description: ValidateAsync loads and executes an agent-provided code script (GetAgentCodeScript + processor.Run(codeScript.Content, ...)), which can lead to remote code execution if untrusted users can modify agent scripts or inputs, unless the code runner is strongly sandboxed (process isolation, restricted FS/network, and strict permissions). RuleCriteria.cs [23-109] Referred Code public async Task<bool> ValidateAsync(Agent agent, IRuleTrigger trigger, CriteriaExecuteOptions options) { if (string.IsNullOrWhiteSpace(agent?.Id)) { return false; } var provider = options.CodeProcessor ?? BuiltInCodeProcessor.PyInterpreter; var processor = _services.GetServices<ICodeProcessor>().FirstOrDefault(x => x.Provider.IsEqualTo(provider)); if (processor == null) { _logger.LogWarning($"Unable to find code processor: {provider}."); return false; } var agentService = _services.GetRequiredService<IAgentService>(); var scriptName = options.CodeScriptName ?? $"{trigger.Name}_rule.py"; var codeScript = await agentService.GetAgentCodeScript(agent.Id, scriptName, scriptType: AgentCodeScriptType.Src); var msg = $"rule trigger ({trigger.Name}) code script ({scriptName}) in agent ({agent.Name}) => args: {options.ArgumentContent?.RootElement.GetRawText()}."; ... (clipped 66 lines)
	Insecure default credentials Description: The repository adds RabbitMQ credentials in configuration with the default guest/guest, which risks credential exposure and insecure deployments if this file (or derived environment) is used beyond local development. appsettings.json [1023-1034] Referred Code "MessageQueue": { "Enabled": false, "Provider": "RabbitMQ" }, "RabbitMQ": { "HostName": "localhost", "Port": 5672, "UserName": "guest", "Password": "guest", "VirtualHost": "/" },
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🔴	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Non-descriptive field: The hook base introduces a publicly mutable field SelfId instead of a clearly named property, reducing self-documentation and API clarity. Referred Code public string SelfId = string.Empty; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Delay calculation bug: The delay conversion uses TimeSpan.FromMilliseconds(qty) for "minute(s)", producing incorrect delays and mishandling boundary/units edge cases. Referred Code case "minute": case "minutes": milliseconds = (long)TimeSpan.FromMilliseconds(qty).TotalMilliseconds; break; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Exposes internal errors: The new controller returns ex.Message in HTTP 500 responses, which can leak internal implementation details to end users. Referred Code catch (Exception ex) { _logger.LogError(ex, "Failed to publish scheduled message"); return StatusCode(StatusCodes.Status500InternalServerError, new PublishMessageResponse { Success = false, Error = ex.Message }); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logs raw payload: The consumer pipeline logs full message bodies (data) which may contain sensitive content and violates secure logging requirements. Referred Code data = Encoding.UTF8.GetString(eventArgs.Body.Span); _logger.LogInformation($"Message received on '{config.QueueName}', id: {eventArgs.BasicProperties?.MessageId}, data: {data}"); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: SSRF risk: The HTTP rule action sends requests to a http_url value taken directly from rule context without validation/allowlisting, enabling SSRF and unsafe outbound requests. Referred Code private string BuildUrl(RuleActionContext context) { Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing user context: The new rule-trigger execution endpoint logs no audit entry and does not capture the authenticated user identity, making it hard to reconstruct who triggered rule actions from audit logs. Referred Code [HttpPost("/rule/trigger/run")] public async Task<IActionResult> RunAction([FromBody] RuleTriggerActionRequest request) { if (request == null) { return BadRequest(new { Success = false, Error = "Request cannnot be empty." }); } var trigger = _services.GetServices<IRuleTrigger>().FirstOrDefault(x => x.Name.IsEqualTo(request.TriggerName)); if (trigger == null) { return BadRequest(new { Success = false, Error = "Unable to find rule trigger." }); } var result = await _ruleEngine.Triggered(trigger, request.Text, request.States, request.Options); return Ok(new { Success = true }); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 2db1aa3

Category	Suggestion	Impact
Possible issue	Remove stray preprocessor directive Remove the stray #endregion preprocessor directive from the ConvertToDictionary method to fix a compilation error. src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs [213-248] private static Dictionary<string, object?> ConvertToDictionary(JsonDocument doc) { var dict = new Dictionary<string, object?>(); foreach (var prop in doc.RootElement.EnumerateObject()) { dict[prop.Name] = prop.Value.ValueKind switch { JsonValueKind.String => prop.Value.GetString(), JsonValueKind.Number when prop.Value.TryGetInt32(out int intValue) => intValue, JsonValueKind.Number when prop.Value.TryGetInt64(out long longValue) => longValue, JsonValueKind.Number when prop.Value.TryGetDouble(out double doubleValue) => doubleValue, JsonValueKind.Number when prop.Value.TryGetDecimal(out decimal decimalValue) => decimalValue, JsonValueKind.Number when prop.Value.TryGetByte(out byte byteValue) => byteValue, JsonValueKind.Number when prop.Value.TryGetSByte(out sbyte sbyteValue) => sbyteValue, JsonValueKind.Number when prop.Value.TryGetUInt16(out ushort uint16Value) => uint16Value, JsonValueKind.Number when prop.Value.TryGetUInt32(out uint uint32Value) => uint32Value, JsonValueKind.Number when prop.Value.TryGetUInt64(out ulong uint64Value) => uint64Value, JsonValueKind.Number when prop.Value.TryGetDateTime(out DateTime dateTimeValue) => dateTimeValue, JsonValueKind.Number when prop.Value.TryGetDateTimeOffset(out DateTimeOffset dateTimeOffsetValue) => dateTimeOffsetValue, JsonValueKind.Number when prop.Value.TryGetGuid(out Guid guidValue) => guidValue, JsonValueKind.Number => prop.Value.GetRawText(), JsonValueKind.True => true, JsonValueKind.False => false, JsonValueKind.Null => null, JsonValueKind.Undefined => null, JsonValueKind.Array => prop.Value, JsonValueKind.Object => prop.Value, _ => prop.Value }; } return dict; - #endregion } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a stray #endregion directive that will cause a compilation error, making this a critical and necessary fix.	High
	Make subscription thread-safe Refactor the SubscribeAsync method to be thread-safe by removing the non-atomic ContainsKey check and relying solely on the result of TryAdd to prevent race conditions. src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs [29-46] public async Task<bool> SubscribeAsync(string key, IMQConsumer consumer) { - if (_consumers.ContainsKey(key)) + var registration = await CreateConsumerRegistrationAsync(consumer); + if (registration == null) { - _logger.LogWarning($"Consumer with key '{key}' is already subscribed."); return false; } - var registration = await CreateConsumerRegistrationAsync(consumer); - if (registration != null && _consumers.TryAdd(key, registration)) + if (!_consumers.TryAdd(key, registration)) { - var config = consumer.Config as RabbitMQConsumerConfig ?? new(); - _logger.LogInformation($"Consumer '{key}' subscribed to queue '{config.QueueName}'."); - return true; + _logger.LogWarning($"Consumer with key '{key}' is already subscribed."); + if (registration.Channel != null) + { + registration.Channel.Dispose(); + } + registration.Consumer.Dispose(); + return false; } - return false; + var config = consumer.Config as RabbitMQConsumerConfig ?? new(); + _logger.LogInformation($"Consumer '{key}' subscribed to queue '{config.QueueName}'."); + return true; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies and fixes a race condition in a concurrent scenario, preventing potential resource leaks and unpredictable behavior, which is a critical improvement for service stability.	High
	Prevent null connection crashes Make the _connection field nullable and add null-checking to its accessors and Dispose method to prevent potential NullReferenceException crashes. src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQConnection.cs [17-41] -private IConnection _connection; +private IConnection? _connection; private bool _disposed = false; public bool IsConnected => _connection != null && _connection.IsOpen && !_disposed; -public IConnection Connection => _connection; +public IConnection Connection => _connection ?? throw new InvalidOperationException("RabbitMQ connection is not established."); Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential NullReferenceException due to an uninitialized non-nullable field and proposes a robust fix, preventing runtime crashes and improving code safety.	Medium
	Robustly parse delay quantity Refactor the ParseDelay method to handle various numeric types for mq_delay_qty to prevent InvalidCastException when parsing rule parameters. src/Infrastructure/BotSharp.Core.Rules/Services/Actions/MessageQueueRuleAction.cs [89-127] private long ParseDelay(RuleActionContext context) { - var qty = (double)context.Parameters.TryGetValueOrDefault("mq_delay_qty", 0); - if (qty == 0) - { - qty = context.Parameters.TryGetValueOrDefault("mq_delay_qty", 0.0); - } + double qty = 0; + + if (context.Parameters.TryGetValue<double>("mq_delay_qty", out var d) && d.HasValue) + qty = d.Value; + else if (context.Parameters.TryGetValue<long>("mq_delay_qty", out var l) && l.HasValue) + qty = l.Value; + else if (context.Parameters.TryGetValue<int>("mq_delay_qty", out var i) && i.HasValue) + qty = i.Value; + else if (context.Parameters.TryGetValue<decimal>("mq_delay_qty", out var dec) && dec.HasValue) + qty = (double)dec.Value; + else if (context.Parameters.TryGetValue<JsonElement>("mq_delay_qty", out var je) && je.HasValue && je.Value.ValueKind == JsonValueKind.Number) + qty = je.Value.GetDouble(); if (qty <= 0) { return 0L; } - var unit = context.Parameters.TryGetValueOrDefault("mq_delay_unit", string.Empty) ?? string.Empty; - unit = unit.ToLower(); + var unit = (context.Parameters.TryGetValueOrDefault("mq_delay_unit", string.Empty) ?? string.Empty).ToLowerInvariant(); - var milliseconds = 0L; - switch (unit) + return unit switch { - case "second": - case "seconds": - milliseconds = (long)TimeSpan.FromSeconds(qty).TotalMilliseconds; - break; - case "minute": - case "minutes": - milliseconds = (long)TimeSpan.FromMinutes(qty).TotalMilliseconds; - break; - case "hour": - case "hours": - milliseconds = (long)TimeSpan.FromHours(qty).TotalMilliseconds; - break; - case "day": - case "days": - milliseconds = (long)TimeSpan.FromDays(qty).TotalMilliseconds; - break; - } - - return milliseconds; + "second" or "seconds" => (long)TimeSpan.FromSeconds(qty).TotalMilliseconds, + "minute" or "minutes" => (long)TimeSpan.FromMinutes(qty).TotalMilliseconds, + "hour" or "hours" => (long)TimeSpan.FromHours(qty).TotalMilliseconds, + "day" or "days" => (long)TimeSpan.FromDays(qty).TotalMilliseconds, + _ => 0L + }; } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential InvalidCastException and provides a much more robust implementation for parsing numeric types, preventing runtime crashes from varied data inputs.	Medium
	Ensure non-null default names Initialize the Name property in AgentRuleConfigBase to string.Empty to prevent potential null reference issues during deserialization or database mapping. src/Infrastructure/BotSharp.Abstraction/Agents/Models/AgentRule.cs [56-67] public class AgentRuleConfigBase { [JsonPropertyName("name")] - public virtual string Name { get; set; } + public virtual string Name { get; set; } = string.Empty; [JsonPropertyName("disabled")] public virtual bool Disabled { get; set; } [JsonPropertyName("config")] [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public virtual JsonDocument? Config { get; set; } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out a potential null reference issue by initializing the Name property, which improves the robustness of the data model against unexpected null values.	Medium
Learned best practice	Guard semaphore release correctly Track whether the semaphore was acquired before releasing it, so cancellation/exception during WaitAsync doesn't incorrectly call Release(). Consider also passing a cancellation token. src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQConnection.cs [52-85] public async Task<bool> ConnectAsync() { - await _lock.WaitAsync(); - + var acquired = false; try { + await _lock.WaitAsync(); + acquired = true; + if (IsConnected) { return true; } var policy = BuildRetryPolicy(); await policy.Execute(async () => { _connection = await _connectionFactory.CreateConnectionAsync(); }); if (IsConnected) { _connection.ConnectionShutdownAsync += OnConnectionShutdownAsync; _connection.CallbackExceptionAsync += OnCallbackExceptionAsync; _connection.ConnectionBlockedAsync += OnConnectionBlockedAsync; _logger.LogInformation($"Rabbit MQ client connection success. host: {_connection.Endpoint.HostName}, port: {_connection.Endpoint.Port}, localPort:{_connection.LocalPort}"); return true; } + _logger.LogError("Rabbit MQ client connection error."); return false; } finally { - _lock.Release(); + if (acquired) _lock.Release(); } - } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - In async/concurrent code, structure lock acquisition so Release() only happens if the lock was successfully acquired to avoid corruption when WaitAsync fails/cancels.	Low
	Add safe defaults for settings Initialize non-nullable configuration properties (or mark them nullable) so Provider can’t be null after binding and comparisons like IsEqualTo(...) don’t crash. src/Infrastructure/BotSharp.Abstraction/Infrastructures/MessageQueues/MessageQueueSettings.cs [3-7] public class MessageQueueSettings { public bool Enabled { get; set; } - public string Provider { get; set; } + public string Provider { get; set; } = string.Empty; } Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Make configuration contracts unambiguous by providing safe defaults / nullability annotations to prevent nulls from configuration binding.	Low
More

---

Previous suggestions

Suggestions up to commit 77c2adb

Category	Suggestion	Impact
Possible issue	Preserve URL replacements Assign the result of url.Replace back to the url variable, as strings are immutable and the method returns a new string with the replacements. src/Infrastructure/BotSharp.Core.Rules/Services/HttpRuleAction.cs [102-112] // Fill in placeholders in url var strValues = context.States.Where(x => x.Value != null && x.Value is string); foreach (var item in strValues) { var value = item.Value as string; if (string.IsNullOrEmpty(value)) { continue; } - url.Replace($"{{{item.Key}}}", value); + url = url.Replace($"{{{item.Key}}}", value); } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug where string immutability is overlooked, causing the url.Replace method to have no effect, and provides the correct fix.	High
General	Return raw JSON body In GetHttpRequestBody, return the body string directly instead of re-serializing it with JsonSerializer.Serialize to avoid corrupting an already-formatted JSON string. src/Infrastructure/BotSharp.Core.Rules/Services/HttpRuleAction.cs [180-189] private string? GetHttpRequestBody(RuleActionContext context) { var body = context.States.TryGetValueOrDefault<string>("http_request_body"); if (string.IsNullOrEmpty(body)) { return null; } - return JsonSerializer.Serialize(body, BotSharpOptions.defaultJsonOptions); + return body; } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that re-serializing an already-JSON string will corrupt it, which is a significant bug, and the proposed fix is correct.	Medium
High-level	Refactor RabbitMQ connection and channel management Simplify the RabbitMQ implementation by removing the custom channel pool. Instead, use a single singleton connection to create short-lived channels on demand, which is a more standard and robust approach. Examples: src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs [198-252] var channelPool = RabbitMQChannelPoolFactory.GetChannelPool(_services, _mqConnection); var channel = channelPool.Get(); try { var args = new Dictionary<string, object?> { ["x-delayed-type"] = "direct" }; ... (clipped 45 lines) src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQChannelPool.cs [1-73] using Microsoft.Extensions.ObjectPool; using RabbitMQ.Client; namespace BotSharp.Plugin.RabbitMQ.Connections; public class RabbitMQChannelPool { private readonly ObjectPool<IChannel> _pool; private readonly ILogger<RabbitMQChannelPool> _logger; private readonly int _tryLimit = 3; ... (clipped 63 lines) Solution Walkthrough: Before: // src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs public async Task<bool> PublishAsync<T>(T payload, MQPublishOptions options) { // ... await policy.Execute(async () => { var channelPool = RabbitMQChannelPoolFactory.GetChannelPool(_services, _mqConnection); var channel = channelPool.Get(); try { // ... publish message using channel } finally { channelPool.Return(channel); } }); // ... } After: // src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs // Remove RabbitMQChannelPool.cs and RabbitMQChannelPoolFactory.cs public async Task<bool> PublishAsync<T>(T payload, MQPublishOptions options) { // ... await policy.Execute(async () => { // Create a short-lived channel directly from the singleton connection using var channel = await _mqConnection.CreateChannelAsync(); // ... publish message using channel // Channel is automatically disposed }); // ... } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies unnecessary complexity in the custom channel pooling and a potential deadlock issue, recommending a simpler, standard practice for RabbitMQ channel management.	Medium
Learned best practice	Copy mutable state dictionaries Avoid sharing the same Dictionary instance across the rule execution and the queued payload by copying context.States into a new dictionary before publishing. src/Infrastructure/BotSharp.Core.Rules/Services/MessageQueueRuleAction.cs [39-47] var payload = new RuleMessagePayload { AgentId = agent.Id, TriggerName = trigger.Name, Channel = trigger.Channel, Text = context.Text, Timestamp = DateTime.UtcNow, - States = context.States + States = context.States is null ? new() : new Dictionary<string, object?>(context.States) }; Suggestion importance[1-10]: 6 __ Why: Relevant best practice - When transferring metadata or other mutable collections between objects, copy the dictionary instead of assigning the same reference to avoid shared-state side effects.	Low
	Avoid blocking on async operations Avoid blocking on the async channel creation inside the pool policy; use the synchronous channel creation API or restructure to create channels asynchronously outside the pool. src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQChannelPool.cs [64-67] public IChannel Create() { - return _connection.CreateChannelAsync().ConfigureAwait(false).GetAwaiter().GetResult(); + return _connection.CreateChannel(); } Suggestion importance[1-10]: 5 __ Why: Relevant best practice - In async/concurrent code, avoid deadlocks by not blocking on async calls (e.g., using GetAwaiter().GetResult()).	Low

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴	Hardcoded credentials Description: The PR introduces RabbitMQ credentials in appsettings.json (RabbitMQ.UserName/RabbitMQ.Password set to guest), which risks leaking or accidentally deploying default credentials and enables unauthorized access if enabled in non-local environments. appsettings.json [1023-1034] Referred Code "MessageQueue": { "Enabled": false, "Provider": "RabbitMQ" }, "RabbitMQ": { "HostName": "localhost", "Port": 5672, "UserName": "guest", "Password": "guest", "VirtualHost": "/" },
⚪	SSRF via HTTP action Description: The HttpRuleAction performs outbound HTTP requests to a URL taken from context.Parameters (http_url) with placeholder substitution and optional headers/body, enabling SSRF (e.g., hitting internal metadata services or internal admin endpoints) if an attacker can influence rule configuration or the parameters passed into rule execution. HttpRuleAction.cs [34-138] Referred Code public async Task<RuleActionResult> ExecuteAsync( Agent agent, IRuleTrigger trigger, RuleActionContext context) { try { var httpMethod = GetHttpMethod(context); if (httpMethod == null) { var errorMsg = $"HTTP method is not supported in agent rule {agent.Name}-{trigger.Name}"; _logger.LogWarning(errorMsg); return RuleActionResult.Failed(errorMsg); } // Build the full URL var fullUrl = BuildUrl(context); using var client = _httpClientFactory.CreateClient(); // Add headers ... (clipped 84 lines)
	User-influenced code execution Description: CodeScriptRuleCriteria selects code_script_name, code_processor, argument_name, and argument_value from context.Parameters which are built from rule config and can be overwritten by incoming states, making it possible for callers of rule triggering endpoints to influence what code script is executed and with what arguments (potentially leading to unintended code execution depending on GetAgentCodeScript and available scripts). CodeScriptRuleCriteria.cs [23-79] Referred Code public async Task<RuleCriteriaResult> ValidateAsync(Agent agent, IRuleTrigger trigger, RuleCriteriaContext context) { var result = new RuleCriteriaResult(); if (string.IsNullOrWhiteSpace(agent?.Id)) { return result; } var provider = context.Parameters.TryGetValueOrDefault<string>("code_processor") ?? BuiltInCodeProcessor.PyInterpreter; var processor = _services.GetServices<ICodeProcessor>().FirstOrDefault(x => x.Provider.IsEqualTo(provider)); if (processor == null) { _logger.LogWarning($"Unable to find code processor: {provider}."); return result; } var agentService = _services.GetRequiredService<IAgentService>(); var scriptName = context.Parameters.TryGetValueOrDefault<string>("code_script_name") ?? $"{trigger.Name}_rule.py"; var codeScript = await agentService.GetAgentCodeScript(agent.Id, scriptName, scriptType: AgentCodeScriptType.Src); ... (clipped 36 lines)
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🔴	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Misspelled method name: The controller action method name UnSubscribeConsuer is misspelled, reducing readability and self-documentation. Referred Code [HttpPost("/message-queue/unsubscribe/consumer")] public async Task<IActionResult> UnSubscribeConsuer([FromBody] UnsubscribeConsumerRequest request) { Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Swallowed deserialization errors: TryGetValue<T> silently swallows JSON deserialization exceptions without logging, making failures hard to diagnose and debug in production. Referred Code public static bool TryGetValue<T>(this IDictionary<string, object?> dict, string key, out T? result, JsonSerializerOptions? jsonOptions = null) { result = default; if (!dict.TryGetValue(key, out var value) || value is null) { return false; } if (value is T t) { result = t; return true; } if (value is JsonElement je) { try { result = je.Deserialize<T>(jsonOptions); return true; ... (clipped 9 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Exception message exposed: The API returns ex.Message to the client on failures, potentially exposing internal implementation details and sensitive information. Referred Code catch (Exception ex) { _logger.LogError(ex, "Failed to publish scheduled message"); return StatusCode(StatusCodes.Status500InternalServerError, new PublishMessageResponse { Success = false, Error = ex.Message }); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logs full message body: The consumer logging includes the full received message data, which can contain sensitive information and violates secure logging practices. Referred Code data = Encoding.UTF8.GetString(eventArgs.Body.Span); _logger.LogInformation($"Message received on '{config.QueueName}', id: {eventArgs.BasicProperties?.MessageId}, data: {data}"); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: SSRF input risk: The HTTP rule action builds and calls a URL from unvalidated context.Parameters (http_url, headers, query params), enabling SSRF and unsafe outbound requests without allowlisting or sanitization. Referred Code // Build the full URL var fullUrl = BuildUrl(context); using var client = _httpClientFactory.CreateClient(); // Add headers AddHttpHeaders(client, context); // Create request var request = new HttpRequestMessage(httpMethod, fullUrl); // Add request body if provided var requestBodyStr = GetHttpRequestBody(context); if (!string.IsNullOrEmpty(requestBodyStr)) { request.Content = new StringContent(requestBodyStr, Encoding.UTF8, MediaTypeNames.Application.Json); } _logger.LogInformation("Executing HTTP rule action for agent {AgentId}, URL: {Url}, Method: {Method}", agent.Id, fullUrl, httpMethod); ... (clipped 21 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing user context: New authenticated endpoints trigger rule actions and MQ operations but do not log the authenticated user identity or outcome context needed to reconstruct who performed the action. Referred Code [HttpPost("/rule/trigger/action")] public async Task<IActionResult> RunAction([FromBody] RuleTriggerActionRequest request) { if (request == null) { return BadRequest(new { Success = false, Error = "Request cannnot be empty." }); } var trigger = _services.GetServices<IRuleTrigger>().FirstOrDefault(x => x.Name.IsEqualTo(request.TriggerName)); if (trigger == null) { return BadRequest(new { Success = false, Error = "Unable to find rule trigger." }); } var result = await _ruleEngine.Triggered(trigger, request.Text, request.States, request.Options); return Ok(new { Success = true }); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

Persistent suggestions updated to latest commit 2db1aa3

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Simplify RabbitMQ connection and channel management The RabbitMQ plugin's custom implementation for connection, channel pooling, and retries is overly complex. It should be replaced by using the RabbitMQ.Client's built-in automatic recovery or a higher-level library like EasyNetQ or MassTransit to simplify the code and increase reliability. Examples: src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQConnection.cs [9-154] public class RabbitMQConnection : IRabbitMQConnection { private readonly RabbitMQSettings _settings; private readonly IConnectionFactory _connectionFactory; private readonly SemaphoreSlim _lock = new(initialCount: 1, maxCount: 1); private readonly ILogger<RabbitMQConnection> _logger; private readonly int _retryCount = 5; private IConnection _connection; private bool _disposed = false; ... (clipped 136 lines) src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs [9-318] Solution Walkthrough: Before: // RabbitMQConnection.cs class RabbitMQConnection { ConnectAsync() { // Use SemaphoreSlim lock // Use Polly to retry connection _connection = _connectionFactory.CreateConnectionAsync(); // Attach manual event handlers for shutdown, exceptions } } // RabbitMQService.cs class RabbitMQService { PublishAsync(...) { // Use Polly to retry publishing // Get channel from custom pool var channelPool = RabbitMQChannelPoolFactory.GetChannelPool(...); var channel = channelPool.Get(); // Manually declare exchange, create properties, and publish channel.BasicPublishAsync(...); // Return channel to pool channelPool.Return(channel); } } After: // Using a higher-level library like EasyNetQ // RabbitMQPlugin.cs public void RegisterDI(IServiceCollection services, IConfiguration config) { // Simplified registration, library handles connection management services.AddEasyNetQ("host=localhost"); services.AddSingleton<IMQService, EasyNetQService>(); } // EasyNetQService.cs class EasyNetQService : IMQService { private readonly IBus _bus; public async Task<bool> PublishAsync<T>(T payload, MQPublishOptions options) { // Publishing becomes a single, simple API call. // The library handles connection, channels, serialization, and retries. await _bus.PubSub.PublishAsync(payload, options.TopicName); return true; } } Suggestion importance[1-10]: 9 __ Why: This is a critical architectural suggestion that correctly identifies the reimplementation of complex connection/channel management, which could be simplified and made more robust by using established libraries or built-in client features.	High
Possible issue	Prevent potential poison message loops Add a check to ensure registration.Channel is open before attempting to BasicAckAsync or BasicNackAsync to prevent potential poison message loops. src/Plugins/BotSharp.Plugin.RabbitMQ/Services/RabbitMQService.cs [147-178] private async Task ConsumeEventAsync(ConsumerRegistration registration, BasicDeliverEventArgs eventArgs) { var data = string.Empty; var config = registration.Consumer.Config as RabbitMQConsumerConfig ?? new(); try { data = Encoding.UTF8.GetString(eventArgs.Body.Span); _logger.LogInformation($"Message received on '{config.QueueName}', id: {eventArgs.BasicProperties?.MessageId}, data: {data}"); var isHandled = await registration.Consumer.HandleMessageAsync(config.QueueName, data); - if (!config.AutoAck && registration.Channel != null) + if (!config.AutoAck && registration.Channel?.IsOpen == true) { if (isHandled) { await registration.Channel.BasicAckAsync(eventArgs.DeliveryTag, multiple: false); } else { await registration.Channel.BasicNackAsync(eventArgs.DeliveryTag, multiple: false, requeue: false); } } } catch (Exception ex) { _logger.LogError(ex, $"Error consuming message on queue '{config.QueueName}': {data}"); - if (!config.AutoAck && registration.Channel != null) + if (!config.AutoAck && registration.Channel?.IsOpen == true) { await registration.Channel.BasicNackAsync(eventArgs.DeliveryTag, multiple: false, requeue: false); } } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies and fixes a potential poison message loop by adding a necessary check for the channel's status before attempting to acknowledge a message, significantly improving error handling robustness.	Medium
	Fix incorrect numeric type conversion Reorder the numeric type checks in ConvertToDictionary to prioritize floating-point types (decimal, double) over integer types to prevent incorrect truncation of numbers. src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs [213-247] private static Dictionary<string, object?> ConvertToDictionary(JsonDocument doc) { var dict = new Dictionary<string, object?>(); foreach (var prop in doc.RootElement.EnumerateObject()) { dict[prop.Name] = prop.Value.ValueKind switch { JsonValueKind.String => prop.Value.GetString(), + // Prioritize floating-point types to avoid truncation + JsonValueKind.Number when prop.Value.TryGetDecimal(out decimal decimalValue) => decimalValue, + JsonValueKind.Number when prop.Value.TryGetDouble(out double doubleValue) => doubleValue, + JsonValueKind.Number when prop.Value.TryGetInt64(out long longValue) => longValue, JsonValueKind.Number when prop.Value.TryGetInt32(out int intValue) => intValue, - JsonValueKind.Number when prop.Value.TryGetInt64(out long longValue) => longValue, - JsonValueKind.Number when prop.Value.TryGetDouble(out double doubleValue) => doubleValue, - JsonValueKind.Number when prop.Value.TryGetDecimal(out decimal decimalValue) => decimalValue, - JsonValueKind.Number when prop.Value.TryGetByte(out byte byteValue) => byteValue, - JsonValueKind.Number when prop.Value.TryGetSByte(out sbyte sbyteValue) => sbyteValue, - JsonValueKind.Number when prop.Value.TryGetUInt16(out ushort uint16Value) => uint16Value, - JsonValueKind.Number when prop.Value.TryGetUInt32(out uint uint32Value) => uint32Value, - JsonValueKind.Number when prop.Value.TryGetUInt64(out ulong uint64Value) => uint64Value, - JsonValueKind.Number when prop.Value.TryGetDateTime(out DateTime dateTimeValue) => dateTimeValue, - JsonValueKind.Number when prop.Value.TryGetDateTimeOffset(out DateTimeOffset dateTimeOffsetValue) => dateTimeOffsetValue, - JsonValueKind.Number when prop.Value.TryGetGuid(out Guid guidValue) => guidValue, - JsonValueKind.Number => prop.Value.GetRawText(), + // Fallback for other numeric types + JsonValueKind.Number => prop.Value.GetDouble(), JsonValueKind.True => true, JsonValueKind.False => false, JsonValueKind.Null => null, JsonValueKind.Undefined => null, + // These will be of type JsonElement, which is acceptable JsonValueKind.Array => prop.Value, JsonValueKind.Object => prop.Value, _ => prop.Value }; } return dict; } Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a data loss bug due to incorrect ordering of type checks and proposes a fix that ensures proper numeric conversions, which is critical for data integrity.	Medium
	Avoid potential deadlocks and improve channel pooling Replace the blocking call in Create() with the synchronous _connection.CreateChannel() and check if the channel is open in Return(IChannel obj) before adding it back to the pool. src/Plugins/BotSharp.Plugin.RabbitMQ/Connections/RabbitMQChannelPool.cs [55-73] internal class ChannelPoolPolicy : IPooledObjectPolicy<IChannel> { private readonly IConnection _connection; public ChannelPoolPolicy(IConnection connection) { _connection = connection; } public IChannel Create() { - return _connection.CreateChannelAsync().ConfigureAwait(false).GetAwaiter().GetResult(); + // CreateChannel() is a synchronous alternative that avoids potential deadlocks. + return _connection.CreateChannel(); } public bool Return(IChannel obj) { - return true; + // Only return the channel to the pool if it's still open. + return obj.IsOpen; } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a potential deadlock issue by avoiding .GetAwaiter().GetResult() and improves the pool's robustness by checking if a channel is open before returning it.	Medium
General	Return real trigger outcome Modify the controller action to return the actual result from _ruleEngine.Triggered, providing clients with meaningful feedback such as success status and new conversation IDs. src/Infrastructure/BotSharp.Core.Rules/Controllers/RuleController.cs [39-40] var result = await _ruleEngine.Triggered(trigger, request.Text, request.States, request.Options); -return Ok(new { Success = true }); +return Ok(new { Success = result != null, ConversationIds = result }); Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly points out that the controller action should return a meaningful result to the client, which is a significant improvement for the API's usability.	Medium
	Improve numeric parameter parsing logic Simplify the ParseDelay method by using context.Parameters.TryGetValueOrDefault("mq_delay_qty") to robustly parse the numeric delay value, removing redundant and error-prone code. src/Infrastructure/BotSharp.Core.Rules/Services/Actions/MessageQueueRuleAction.cs [89-127] private long ParseDelay(RuleActionContext context) { - var qty = (double)context.Parameters.TryGetValueOrDefault("mq_delay_qty", 0); - if (qty == 0) - { - qty = context.Parameters.TryGetValueOrDefault("mq_delay_qty", 0.0); - } + var qty = context.Parameters.TryGetValueOrDefault<double>("mq_delay_qty"); if (qty <= 0) { return 0L; } var unit = context.Parameters.TryGetValueOrDefault("mq_delay_unit", string.Empty) ?? string.Empty; unit = unit.ToLower(); var milliseconds = 0L; switch (unit) { case "second": case "seconds": milliseconds = (long)TimeSpan.FromSeconds(qty).TotalMilliseconds; break; case "minute": case "minutes": milliseconds = (long)TimeSpan.FromMinutes(qty).TotalMilliseconds; break; case "hour": case "hours": milliseconds = (long)TimeSpan.FromHours(qty).TotalMilliseconds; break; case "day": case "days": milliseconds = (long)TimeSpan.FromDays(qty).TotalMilliseconds; break; } return milliseconds; } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies flawed and redundant logic for parsing a numeric parameter and provides a cleaner, more robust solution using the generic extension method.	Low
Learned best practice	Avoid shared mutable dictionaries Copy context.Parameters into a new dictionary (or an immutable representation) before assigning to the payload so later mutations to the context don't affect the published message. src/Infrastructure/BotSharp.Core.Rules/Services/Actions/MessageQueueRuleAction.cs [37-45] var payload = new RuleMessagePayload { AgentId = agent.Id, TriggerName = trigger.Name, Channel = trigger.Channel, Text = context.Text, Timestamp = DateTime.UtcNow, - States = context.Parameters + States = context.Parameters == null ? new Dictionary<string, object?>() : new Dictionary<string, object?>(context.Parameters) }; Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - When transferring metadata or other mutable collections between objects, copy the dictionary instead of assigning the same reference to avoid shared-state side effects.	Low
More