Skip to content

fix lazy mode#1312

Merged
Oceania2018 merged 1 commit intomasterfrom
lazy-mode-fixing
Mar 19, 2026
Merged

fix lazy mode#1312
Oceania2018 merged 1 commit intomasterfrom
lazy-mode-fixing

Conversation

@Oceania2018
Copy link
Member

No description provided.

@Oceania2018 Oceania2018 merged commit ad96e08 into master Mar 19, 2026
1 of 5 checks passed
@qodo-code-review
Copy link
Contributor

Review Summary by Qodo

Fix lazy routing mode agent selection and instruction flow

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix lazy routing mode by checking if lazy routing agent ID exists before using it
• Restructure routing logic to call appropriate instruction method based on routing mode
• Simplify lazy routing agent update logic by removing redundant mode check
• Reload agent after setting lazy routing agent ID to ensure correct agent state
Diagram
flowchart LR
  A["Routing Agent Request"] --> B["Push Agent to Context"]
  B --> C["Check Routing Mode"]
  C --> D{"Lazy Mode & Agent ID Exists?"}
  D -->|Yes| E["Load Lazy Agent"]
  E --> F["Call InstructDirect"]
  D -->|No| G["Call InstructLoop"]
  H["UpdateLazyRoutingAgent"] --> I{"Update Flag True?"}
  I -->|Yes| J["Set Lazy Agent ID"]
  I -->|No| K["Skip Update"]
Loading

Grey Divider

File Changes

1. src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs 🐞 Bug fix +11/-5

Add lazy routing agent validation and conditional instruction routing

• Reorder context push before routing mode check for consistency
• Add null/empty check for lazy routing agent ID before using it
• Load the lazy routing agent after setting CurrentAgentId
• Split routing instruction logic into conditional branches based on routing mode
• Call InstructDirect for lazy mode and InstructLoop for eager mode

src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs


2. src/Infrastructure/BotSharp.Core/Routing/RoutingContext.cs 🐞 Bug fix +3/-7

Simplify lazy routing agent update logic

• Remove redundant routing mode check in UpdateLazyRoutingAgent method
• Simplify logic to always set lazy routing agent ID when not fallback agent
• Reduce conditional nesting by removing mode validation

src/Infrastructure/BotSharp.Core/Routing/RoutingContext.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 19, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. LoadAgent null dereferenced 📘 Rule violation ⛯ Reliability
Description
In lazy routing, the code calls agentService.LoadAgent and then passes agent to
routing.InstructDirect without guarding against LoadAgent returning null, which can cause a
NullReferenceException at runtime. This violates the requirement to add explicit null/empty guards
at provider/storage/state boundaries and return a safe fallback with logging.
Code

src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs[R92-95]

+                    message.CurrentAgentId = lazyRoutingAgentId;
+                    agent = await agentService.LoadAgent(message.CurrentAgentId);
                    await routing.Context.Push(message.CurrentAgentId, reason: "lazy routing", updateLazyRouting: false);
+                    response = await routing.InstructDirect(agent, message, dialogs);
Evidence
PR Compliance ID 2 requires null/empty guards at boundaries to avoid NREs. The new lazy routing
branch loads an agent from state-derived lazyRoutingAgentId and immediately dereferences it via
InstructDirect; LoadAgent explicitly returns null for empty/unknown IDs, and InstructDirect
dereferences agent.Id/agent.Disabled without null checks.

src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs[92-95]
src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.LoadAgent.cs[12-34]
src/Infrastructure/BotSharp.Core/Routing/RoutingService.cs[29-47]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`agentService.LoadAgent(message.CurrentAgentId)` can return `null`, but the lazy-routing path passes the possibly-null `agent` into `routing.InstructDirect`, which dereferences `agent` and can throw.

## Issue Context
`lazyRoutingAgentId` is loaded from conversation state and may be stale/invalid. `AgentService.LoadAgent` returns `null` when the ID is empty/invalid or the agent does not exist.

## Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs[90-100]
- src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.LoadAgent.cs[12-34]
- src/Infrastructure/BotSharp.Core/Routing/RoutingService.cs[29-47]

## Implementation notes
- Load into a separate variable (e.g., `lazyAgent`) and check for null.
- If null, log a clear warning (without sensitive content) and fall back to a safe behavior (e.g., keep the current `agent` and call `InstructLoop`, or return a generic failure response).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +92 to +95
message.CurrentAgentId = lazyRoutingAgentId;
agent = await agentService.LoadAgent(message.CurrentAgentId);
await routing.Context.Push(message.CurrentAgentId, reason: "lazy routing", updateLazyRouting: false);
response = await routing.InstructDirect(agent, message, dialogs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. loadagent null dereferenced 📘 Rule violation ⛯ Reliability

In lazy routing, the code calls agentService.LoadAgent and then passes agent to
routing.InstructDirect without guarding against LoadAgent returning null, which can cause a
NullReferenceException at runtime. This violates the requirement to add explicit null/empty guards
at provider/storage/state boundaries and return a safe fallback with logging.
Agent Prompt
## Issue description
`agentService.LoadAgent(message.CurrentAgentId)` can return `null`, but the lazy-routing path passes the possibly-null `agent` into `routing.InstructDirect`, which dereferences `agent` and can throw.

## Issue Context
`lazyRoutingAgentId` is loaded from conversation state and may be stale/invalid. `AgentService.LoadAgent` returns `null` when the ID is empty/invalid or the agent does not exist.

## Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs[90-100]
- src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.LoadAgent.cs[12-34]
- src/Infrastructure/BotSharp.Core/Routing/RoutingService.cs[29-47]

## Implementation notes
- Load into a separate variable (e.g., `lazyAgent`) and check for null.
- If null, log a clear warning (without sensitive content) and fall back to a safe behavior (e.g., keep the current `agent` and call `InstructLoop`, or return a generic failure response).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant