Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Oct 30, 2025

PR Type

Enhancement


Description

  • Refine rule engine with code script execution support

  • Add agent rule output arguments and trigger options

  • Introduce coding settings for LLM provider configuration

  • Restructure code processor and rule trigger interfaces


Diagram Walkthrough

flowchart LR
  A["Rule Engine"] -->|"Execute Code Script"| B["Code Processor"]
  A -->|"Trigger Options"| C["RuleTriggerOptions"]
  C -->|"Contains"| D["Arguments & States"]
  E["Agent Settings"] -->|"Includes"| F["Coding Settings"]
  F -->|"Configures"| B
  G["IRuleTrigger"] -->|"Provides"| H["OutputArgs"]
Loading

File Walkthrough

Relevant files
Configuration changes
5 files
AgentSettings.cs
Add coding settings to agent configuration                             
+6/-2     
Using.cs
Add global using statements for new namespaces                     
+3/-1     
Using.cs
Add coding settings to global using statements                     
+1/-0     
Using.cs
Add coding enums and settings to global using                       
+3/-0     
appsettings.json
Add coding settings configuration section                               
+4/-0     
Enhancement
14 files
BuiltInCodeProcessor.cs
Create built-in code processor constants                                 
+6/-0     
CodeGenerationOptions.cs
Rename language property to programming language                 
+3/-4     
CodingSettings.cs
New coding settings for LLM configuration                               
+14/-0   
IRuleEngine.cs
Refactor rule engine interface with new signature               
+10/-1   
IRuleTrigger.cs
Add output arguments to rule trigger interface                     
+7/-0     
RuleTriggerOptions.cs
New rule trigger options for code execution                           
+31/-0   
RuleEngine.cs
Implement code script execution in rule engine                     
+105/-33
AgentService.Coding.cs
Use built-in code processor constant                                         
+2/-1     
CodingPlugin.cs
New plugin for coding functionality                                           
+15/-0   
InstructService.Execute.cs
Use built-in code processor constant                                         
+2/-1     
AgentController.Coding.cs
Update state setting for programming language                       
+2/-1     
AgentController.Rule.cs
Return rule view model with output arguments                         
+4/-3     
AgentRuleViewModel.cs
New view model for agent rule with output args                     
+29/-0   
PyCodeInterpreter.cs
Add agent settings and LLM provider configuration               
+27/-8   
Refactoring
1 files
AgentPlugin.cs
Remove code script executor from agent plugin                       
+0/-3     
Bug fix
1 files
GoogleController.cs
Fix logger injection and parameter ordering                           
+6/-3     

@iceljc iceljc marked this pull request as draft October 30, 2025 17:05
@qodo-merge-pro
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Limited auditing: New critical actions like code script execution and rule triggering are only partially
logged (warnings/info) without consistent audit entries including user/context, making it
unclear if full audit trail requirements are met.

Referred Code
#region Private methods
private async Task<bool> TriggerCodeScript(string agentId, string triggerName, RuleTriggerOptions options)
{
    if (string.IsNullOrWhiteSpace(agentId))
    {
        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 ?? $"{triggerName}_rule.py";
    var codeScript = await agentService.GetAgentCodeScript(agentId, scriptName, scriptType: AgentCodeScriptType.Src);

    var msg = $"rule trigger ({triggerName}) code script ({scriptName}) in agent ({agentId}) => args: {options.Arguments?.RootElement.GetRawText()}.";


 ... (clipped 54 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Sparse error context: Exceptions from code script execution are caught and logged but without including
operation context like script name/provider/args in structured form, and earlier null
cases return false without clear upstream handling.

Referred Code
try
{
    var response = await processor.RunAsync(codeScript, options: new()
    {
        ScriptName = scriptName,
        Arguments = BuildArguments(options.ArgsName, options.Arguments)
    });

    if (response == null || !response.Success)
    {
        _logger.LogWarning($"Failed to handle {msg}");
        return false;
    }

    bool result;
    LogLevel logLevel;
    if (response.Result.IsEqualTo("true") || response.Result.IsEqualTo("1"))
    {
        logLevel = LogLevel.Information;
        result = true;
    }


 ... (clipped 16 lines)
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:
Args logged raw: The code logs raw arguments from JsonDocument via GetRawText which may include sensitive
data, without redaction or classification, potentially exposing secrets in logs.

Referred Code
var msg = $"rule trigger ({triggerName}) code script ({scriptName}) in agent ({agentId}) => args: {options.Arguments?.RootElement.GetRawText()}.";

if (string.IsNullOrWhiteSpace(codeScript))
{
    _logger.LogWarning($"Unable to find {msg}.");
    return false;
}

try
{
    var response = await processor.RunAsync(codeScript, options: new()
    {
        ScriptName = scriptName,
        Arguments = BuildArguments(options.ArgsName, options.Arguments)
    });

    if (response == null || !response.Success)
    {
        _logger.LogWarning($"Failed to handle {msg}");
        return false;
    }


 ... (clipped 16 lines)
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated input: Code script execution consumes Arguments and ArgsName without validation or sanitization,
and scriptName defaults from triggerName without normalization, which may pose
injection/path risks depending on downstream implementations.

Referred Code
#region Private methods
private async Task<bool> TriggerCodeScript(string agentId, string triggerName, RuleTriggerOptions options)
{
    if (string.IsNullOrWhiteSpace(agentId))
    {
        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 ?? $"{triggerName}_rule.py";
    var codeScript = await agentService.GetAgentCodeScript(agentId, scriptName, scriptType: AgentCodeScriptType.Src);

    var msg = $"rule trigger ({triggerName}) code script ({scriptName}) in agent ({agentId}) => args: {options.Arguments?.RootElement.GetRawText()}.";


 ... (clipped 22 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consider a more flexible rule evaluation

The current rule engine only triggers based on code script execution. It should
be enhanced to also support the previous method of using an LLM to evaluate
natural language criteria, making the engine more versatile.

Examples:

src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs [43-56]
        foreach (var agent in filteredAgents)
        {
            var isTriggered = true;

            // Code trigger
            if (options != null)
            {
                isTriggered = await TriggerCodeScript(agent.Id, trigger.Name, options);
            }


 ... (clipped 4 lines)

Solution Walkthrough:

Before:

// In RuleEngine.cs
public async Task<IEnumerable<string>> Trigger(IRuleTrigger trigger, string text, RuleTriggerOptions? options = null)
{
    // ... get agents ...
    foreach (var agent in filteredAgents)
    {
        var isTriggered = true;

        // Trigger is determined ONLY by code script execution.
        if (options != null)
        {
            isTriggered = await TriggerCodeScript(agent.Id, trigger.Name, options);
        }

        if (isTriggered)
        {
            // Create new conversation...
        }
    }
    return newConversationIds;
}

After:

// In RuleEngine.cs
public async Task<IEnumerable<string>> Trigger(IRuleTrigger trigger, string text, RuleTriggerOptions? options = null)
{
    // ... get agents ...
    foreach (var agent in filteredAgents)
    {
        bool isTriggered = false;

        // Support both code-based and natural language-based rule evaluation.
        if (options != null && !string.IsNullOrEmpty(options.CodeScriptName))
        {
            isTriggered = await TriggerCodeScript(agent.Id, trigger.Name, options);
        }
        else // Fallback to natural language evaluation
        {
            isTriggered = await EvaluateNaturalLanguageCriteria(agent, text);
        }

        if (isTriggered)
        {
            // Create new conversation...
        }
    }
    return newConversationIds;
}
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the PR replaces a flexible, LLM-based rule evaluation with a code-only approach, and proposes a more versatile design supporting both, which would significantly enhance the rule engine's power and usability.

Medium
Possible issue
Dispose JsonDocument to prevent memory leaks

In the BuildArguments method, wrap the JsonDocument parameter args in a using
statement to ensure it is properly disposed and prevent potential memory leaks.

src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs [157-165]

 private IEnumerable<KeyValue> BuildArguments(string? argName, JsonDocument? args)
 {
     var keyValues = new List<KeyValue>();
     if (args != null)
     {
-        keyValues.Add(new KeyValue(argName ?? "rule_args", args.RootElement.GetRawText()));
+        using (args)
+        {
+            keyValues.Add(new KeyValue(argName ?? "rule_args", args.RootElement.GetRawText()));
+        }
     }
     return keyValues;
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential memory leak by not disposing the JsonDocument instance and provides a valid fix, improving resource management.

Medium
General
Improve fallback logic for LLM configuration

Improve the GetLlmProviderModel method's fallback logic. Before resorting to
hardcoded values, attempt to use the agent's default LLM configuration from
_agentSettings.LlmConfig.

src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [186-200]

 private (string, string) GetLlmProviderModel()
 {
     var provider = _agentSettings.Coding?.Provider;
     var model = _agentSettings.Coding?.Model;
 
     if (!string.IsNullOrEmpty(provider) && !string.IsNullOrEmpty(model))
     {
         return (provider, model);
     }
 
-    provider = "openai";
-    model = "gpt-5-mini";
+    // Fallback to agent's default LLM config
+    if (!string.IsNullOrEmpty(_agentSettings.LlmConfig?.Provider) && !string.IsNullOrEmpty(_agentSettings.LlmConfig?.Model))
+    {
+        return (_agentSettings.LlmConfig.Provider, _agentSettings.LlmConfig.Model);
+    }
 
-    return (provider, model);
+    // Fallback to hardcoded values if no config is available
+    return ("openai", "gpt-5-mini");
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion proposes a more robust fallback mechanism by using the agent's general LLM configuration before resorting to hardcoded values, which improves the configuration logic's flexibility and consistency.

Low
Learned
best practice
Cache default JsonDocument

Avoid parsing a new JsonDocument on every property access and return a cached,
safely created instance. Guard against exceptions and unnecessary allocations.

src/Infrastructure/BotSharp.Abstraction/Rules/IRuleTrigger.cs [18]

-JsonDocument OutputArgs => JsonDocument.Parse("{}");
+private static readonly JsonDocument EmptyArgs = JsonDocument.Parse("{}");
+JsonDocument OutputArgs => EmptyArgs;
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Ensure nullability guards before property access and default to safe fallbacks when inputs can be missing.

Low
  • More

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant