Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Jan 7, 2026

PR Type

Enhancement


Description

  • Upgrade target framework from .NET 8.0 to .NET 10.0

  • Reorganize MultiTenancy plugin namespaces for better structure

  • Improve code formatting and null-safety checks

  • Refactor variable naming conventions for consistency


Diagram Walkthrough

flowchart LR
  A["Target Framework"] -->|"net8.0 to net10.0"| B["Directory.Build.props"]
  C["MultiTenancy Plugin"] -->|"Reorganize namespaces"| D["Providers & Tenant folders"]
  E["Code Quality"] -->|"Improve formatting & null-safety"| F["Multiple files"]
  G["Variable Naming"] -->|"Consistency improvements"| H["A2A services"]
Loading

File Walkthrough

Relevant files
Configuration changes
10 files
Directory.Build.props
Update target framework to .NET 10.0                                         
+1/-1     
MultiTenancyServiceCollectionExtensions.cs
Add missing namespace imports for reorganized structure   
+3/-0     
ICurrentTenantAccessor.cs
Move interface to dedicated Interfaces namespace                 
+1/-1     
ConfigTenantOptionProvider.cs
Update namespace to Providers folder structure                     
+1/-1     
DefaultConnectionStringResolver.cs
Reorganize namespace and improve code formatting                 
+6/-2     
TenantConnectionProvider.cs
Update namespace and improve conditional formatting           
+5/-2     
AsyncLocalCurrentTenantAccessor.cs
Update namespace and add interface import                               
+2/-1     
CurrentTenant.cs
Update namespace and add interface import                               
+2/-1     
TenantFeature.cs
Update namespace to Tenant folder structure                           
+1/-1     
TenantResolver.cs
Update namespace and improve code formatting                         
+5/-2     
Formatting
4 files
TenantConfiguration.cs
Improve code formatting and exception handling                     
+10/-3   
A2AService.cs
Standardize parameter naming convention                                   
+2/-2     
A2ASettings.cs
Remove unused using statements                                                     
+0/-6     
MongoStoragePlugin.cs
Improve code formatting for conditional statements             
+4/-1     
Enhancement
1 files
A2AAgentHook.cs
Refactor variable naming and add null-safety checks           
+38/-35 

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
SSRF via endpoint

Description: The new code fetches remote capabilities using
_a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint) where remoteConfig.Endpoint
appears to come from settings (_a2aSettings.Agents) without validation/allowlisting,
creating a realistic SSRF risk (e.g., pointing the endpoint to internal services like
http://169.254.169.254/ or other intranet hosts).
A2AAgentHook.cs [45-56]

Referred Code
var remoteConfig = _a2aSettings.Agents?.FirstOrDefault(x => x.Id == agent.Id);
if (remoteConfig != null)
{
    var agentCard = _a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint).GetAwaiter().GetResult();
    if (agentCard != null)
    {
        agent.Name = agentCard.Name;
        agent.Description = agentCard.Description;
        agent.Instruction = $"You are a proxy interface for an external intelligent service named '{agentCard.Name}'. " +
                            $"Your ONLY goal is to forward the user's request verbatim to the external service. " +
                            $"You must use the function 'delegate_to_a2a' to communicate with it. " +
                            $"Do not attempt to answer the question yourself.";
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

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:
Unhandled remote failure: The new synchronous wait on GetCapabilitiesAsync(...).GetAwaiter().GetResult() has no
exception handling or fallback, so network/timeouts can crash agent loading without
actionable context or graceful degradation.

Referred Code
var agentCard = _a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint).GetAwaiter().GetResult();
if (agentCard != null)
{
    agent.Name = agentCard.Name;
    agent.Description = agentCard.Description;
    agent.Instruction = $"You are a proxy interface for an external intelligent service named '{agentCard.Name}'. " +
                        $"Your ONLY goal is to forward the user's request verbatim to the external service. " +
                        $"You must use the function 'delegate_to_a2a' to communicate with it. " +
                        $"Do not attempt to answer the question yourself.";

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 logging: The new remote-agent enrichment flow performs a remote capability fetch and mutates agent
without any audit trail that would allow reconstructing who/what triggered the change and
whether it succeeded.

Referred Code
var remoteConfig = _a2aSettings.Agents?.FirstOrDefault(x => x.Id == agent.Id);
if (remoteConfig != null)
{
    var agentCard = _a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint).GetAwaiter().GetResult();
    if (agentCard != null)
    {
        agent.Name = agentCard.Name;
        agent.Description = agentCard.Description;
        agent.Instruction = $"You are a proxy interface for an external intelligent service named '{agentCard.Name}'. " +
                            $"Your ONLY goal is to forward the user's request verbatim to the external service. " +
                            $"You must use the function 'delegate_to_a2a' to communicate with it. " +
                            $"Do not attempt to answer the question yourself.";

        var properties = new Dictionary<string, object>
        {
            {
                "user_query",
                new
                {
                    type = "string",
                    description = "The exact user request or task description to be forwarded."


 ... (clipped 19 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 exposure risk: The new ArgumentException messages may bubble to user-facing surfaces depending on how
TenantConfiguration is constructed/validated, which requires verification that these are
not returned directly to end users.

Referred Code
    if (string.IsNullOrWhiteSpace(name))
    {
        throw new ArgumentException("Name cannot be null or whitespace.");
    }

    Id = id;
    Name = name;
    ConnectionStrings = new ConnectionStrings();
}

public TenantConfiguration(Guid id, [NotNull] string name, [NotNull] string normalizedName)
    : this(id, name)
{
    if (string.IsNullOrWhiteSpace(normalizedName))
    {
        throw new ArgumentException("NormalizedName cannot be null or whitespace.");
    }

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:
No safe diagnostics: The new remote capability call and agent mutation add operational risk without any
structured internal logging to help diagnose failures while ensuring no sensitive data is
logged.

Referred Code
var remoteConfig = _a2aSettings.Agents?.FirstOrDefault(x => x.Id == agent.Id);
if (remoteConfig != null)
{
    var agentCard = _a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint).GetAwaiter().GetResult();
    if (agentCard != null)
    {
        agent.Name = agentCard.Name;
        agent.Description = agentCard.Description;
        agent.Instruction = $"You are a proxy interface for an external intelligent service named '{agentCard.Name}'. " +
                            $"Your ONLY goal is to forward the user's request verbatim to the external service. " +
                            $"You must use the function 'delegate_to_a2a' to communicate with it. " +
                            $"Do not attempt to answer the question yourself.";

        var properties = new Dictionary<string, object>
        {
            {
                "user_query",
                new
                {
                    type = "string",
                    description = "The exact user request or task description to be forwarded."


 ... (clipped 19 lines)

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:
External endpoint validation: The new code uses remoteConfig.Endpoint to reach an external service without visible
validation/allowlisting, so verification is needed that endpoints are trusted and
SSRF-style risks are mitigated elsewhere.

Referred Code
var remoteConfig = _a2aSettings.Agents?.FirstOrDefault(x => x.Id == agent.Id);
if (remoteConfig != null)
{
    var agentCard = _a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint).GetAwaiter().GetResult();
    if (agentCard != null)

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
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
The .NET 10 upgrade is incomplete

The upgrade to .NET 10 is incomplete as it only modifies the target framework
property. This major version jump necessitates dependency updates and code
changes to handle breaking API changes, which are currently absent.

Examples:

Directory.Build.props [3]
        <TargetFramework>net10.0</TargetFramework>

Solution Walkthrough:

Before:

// Directory.Build.props
<Project>
    <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <LangVersion>12.0</LangVersion>
        ...
    </PropertyGroup>
</Project>

After:

// Directory.Build.props
<Project>
    <PropertyGroup>
        <TargetFramework>net10.0</TargetFramework>
        <LangVersion>12.0</LangVersion>
        ...
    </PropertyGroup>
</Project>

// Other project/solution files
// Missing:
// - Updates to NuGet package dependencies
// - Code fixes for breaking API changes
// - Updates to global.json if used
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical flaw; upgrading the .NET framework from net8.0 to net10.0 with a single line change is insufficient and will likely cause build failures and instability.

High
Possible issue
Avoid blocking on async calls

Refactor the OnAgentLoaded method to be asynchronous by using async Task and
await. This avoids blocking on the GetCapabilitiesAsync call, preventing
potential deadlocks.

src/Infrastructure/BotSharp.Core.A2A/Hooks/A2AAgentHook.cs [37-87]

-    public override void OnAgentLoaded(Agent agent)
+    public override async Task OnAgentLoaded(Agent agent)
     {
         // Check if this is an A2A remote agent
         if (agent.Type != AgentType.A2ARemote)
         {
             return;
         }
 
         var remoteConfig = _a2aSettings.Agents?.FirstOrDefault(x => x.Id == agent.Id);
         if (remoteConfig != null)
         {
-            var agentCard = _a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint).GetAwaiter().GetResult();
+            var agentCard = await _a2aService.GetCapabilitiesAsync(remoteConfig.Endpoint);
             if (agentCard != null)
             {
                 agent.Name = agentCard.Name;
                 agent.Description = agentCard.Description;
 ...
                 agent.Functions.Add(new FunctionDef
                 {
 ...
                 });
             }
         }
-        base.OnAgentLoaded(agent);
+        await base.OnAgentLoaded(agent);
     }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a blocking async call (.GetAwaiter().GetResult()) which can cause deadlocks, and proposes the best practice of using async/await. This is a significant improvement for application stability.

Medium
Learned
best practice
Add missing null collection guard

Guard against _tenantStoreOptions.Tenants being null before calling .Any() to
avoid potential NullReferenceException.

src/Plugins/BotSharp.Plugin.MultiTenancy/MultiTenancy/Providers/DefaultConnectionStringResolver.cs [22-25]

-if (!_tenantStoreOptions.Enabled || !_tenantStoreOptions.Tenants.Any())
+if (!_tenantStoreOptions.Enabled || _tenantStoreOptions.Tenants == null || !_tenantStoreOptions.Tenants.Any())
 {
     return null;
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Improve defensive coding with precise null and state checks to prevent crashes from null collections or invalid state.

Low
  • More

@iceljc iceljc merged commit 3380cfb into SciSharp:master Jan 7, 2026
4 checks passed
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