Skip to content

fix(AgentController): case-insensitive agentId filtering and null-guard for GetRuleTriggers#1343

Merged
Oceania2018 merged 10 commits intoSciSharp:masterfrom
visagang:features/vguruparan
May 4, 2026
Merged

fix(AgentController): case-insensitive agentId filtering and null-guard for GetRuleTriggers#1343
Oceania2018 merged 10 commits intoSciSharp:masterfrom
visagang:features/vguruparan

Conversation

@visagang
Copy link
Copy Markdown
Contributor

@visagang visagang commented May 4, 2026

Summary

Hardens the new GET /rule/triggers/{agentId} endpoint in AgentController.Rule.cs so that:

  1. The agentId route parameter is matched case-insensitively against IRuleTrigger.AgentIds.
  2. Null, empty, or whitespace agentId values no longer cause a silent over-restrictive filter; the endpoint now falls back to returning all registered triggers.

Motivation

  • agentId is an identifier and should behave consistently regardless of casing supplied by the caller. The previous implementation used the default IEnumerable<string>.Contains overload, which performs an ordinal, case-sensitive comparison and could exclude valid triggers depending on how the caller cased the route segment.
  • The endpoint is an API boundary and previously did no input validation on agentId. An empty/whitespace value would be passed straight into the Where(...) filter and would almost certainly match nothing, producing a misleading empty result instead of a safe, predictable response.

Changes

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs

  • Switched the trigger filter to x.AgentIds.Contains(agentId, StringComparer.OrdinalIgnoreCase) so identifier matching is ordinal and case-insensitive.
  • Added a string.IsNullOrWhiteSpace(agentId) guard; when the parameter is missing or blank, the agent filter is skipped and all triggers are returned (consistent with the parameterless GET /rule/triggers overload).
  • No changes to method signature, route, return type, or DI/constructor — purely an in-method behavior fix.

Risk / Compatibility

  • Backwards compatible for correctly-cased calls.
  • Callers that previously relied on case-sensitive exclusion will now see additional triggers returned for differently-cased ids; this matches the stated identifier semantics and the behavior of related endpoints.
  • No public API, schema, or DI changes.

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add case-insensitive agent filtering to rule triggers endpoint

✨ Enhancement 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Add optional agentId route parameter to filter rule triggers
• Implement case-insensitive agent ID matching using StringComparer.OrdinalIgnoreCase
• Add null-guard to handle empty/whitespace agentId values safely
• Introduce AgentIds property to IRuleTrigger interface for UI filtering
Diagram
flowchart LR
  A["IRuleTrigger interface"] -->|"Add AgentIds property"| B["AgentIds: string[]?"]
  C["GET /rule/triggers/{agentId}"] -->|"New endpoint"| D["Filter triggers by agentId"]
  D -->|"Case-insensitive match"| E["StringComparer.OrdinalIgnoreCase"]
  D -->|"Null-guard check"| F["IsNullOrWhiteSpace validation"]
  F -->|"If empty"| G["Return all triggers"]
  F -->|"If valid"| H["Filter by AgentIds"]
Loading

Grey Divider

File Changes

1. src/Infrastructure/BotSharp.Abstraction/Rules/IRuleTrigger.cs ✨ Enhancement +6/-0

Add optional AgentIds property to IRuleTrigger

• Add new optional AgentIds property to IRuleTrigger interface
• Property returns null by default for backward compatibility
• Documented as UI display/filtering only, not used in execution logic

src/Infrastructure/BotSharp.Abstraction/Rules/IRuleTrigger.cs


2. src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs ✨ Enhancement +17/-0

Add case-insensitive agent ID filtering endpoint

• Add new GetRuleTriggers(string agentId) overload endpoint at GET /rule/triggers/{agentId}
• Implement case-insensitive filtering using StringComparer.OrdinalIgnoreCase
• Add null/whitespace guard to return all triggers when agentId is empty
• Filter logic includes triggers with null AgentIds or matching agent IDs

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 4, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Filtered route returns unfiltered🐞 Bug ≡ Correctness
Description
GetRuleTriggers(string agentId) skips filtering when agentId is whitespace, so calls to the filtered
route /rule/triggers/{agentId} can return the full unfiltered trigger list. This makes invalid route
values indistinguishable from the explicit /rule/triggers endpoint and can hide client/request bugs.
Code

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs[R8-15]

+    [HttpGet("/rule/triggers/{agentId}")]
+    public IEnumerable<AgentRuleViewModel> GetRuleTriggers(string agentId)
+    {
+        var triggers = _services.GetServices<IRuleTrigger>();
+        if (!string.IsNullOrWhiteSpace(agentId))
+        {
+            triggers = triggers.Where(x => x.AgentIds == null || x.AgentIds.Contains(agentId, StringComparer.OrdinalIgnoreCase));
+        }
Evidence
The new action is bound to a non-optional route segment ("/rule/triggers/{agentId}") but explicitly
disables the agentId filter when the provided value is whitespace; since there is also a separate
unfiltered GET /rule/triggers action, the filtered path can unexpectedly behave like the unfiltered
one for invalid inputs.

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs[8-16]
src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs[25-36]

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

## Issue description
`GET /rule/triggers/{agentId}` currently returns *all* triggers when `agentId` is whitespace, making the filtered route behave like the unfiltered `GET /rule/triggers` endpoint for invalid inputs.
### Issue Context
This endpoint already has an explicit unfiltered overload (`GET /rule/triggers`). A whitespace/invalid `{agentId}` should be treated as a client error (e.g., 400) or the endpoint should always apply filtering.
### Fix Focus Areas
- src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs[8-15]
### Suggested change
- Replace the `if (!string.IsNullOrWhiteSpace(agentId)) { ... }` guard with:
- `if (string.IsNullOrWhiteSpace(agentId)) return BadRequest("agentId is required");` (change return type to `ActionResult<IEnumerable<AgentRuleViewModel>>`), **or**
- remove the guard and always apply the filter for this route.

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



Advisory comments

2. AgentIds evaluated twice🐞 Bug ⚙ Maintainability
Description
The filter predicate reads x.AgentIds twice, which is avoidable and can cause redundant work if an
IRuleTrigger implementation computes AgentIds on access. Storing the property value in a local
within the predicate prevents repeated evaluation.
Code

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs[14]

+            triggers = triggers.Where(x => x.AgentIds == null || x.AgentIds.Contains(agentId, StringComparer.OrdinalIgnoreCase));
Evidence
The predicate checks x.AgentIds == null and then re-reads x.AgentIds for .Contains(...). Since
AgentIds is an interface member that may be overridden, repeated access is not guaranteed to be
free/stable.

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs[12-15]
src/Infrastructure/BotSharp.Abstraction/Rules/IRuleTrigger.cs[5-30]

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

## Issue description
The LINQ predicate reads `x.AgentIds` twice.
### Issue Context
`AgentIds` is an interface property and could be overridden with a computed getter; caching improves defensiveness and readability.
### Fix Focus Areas
- src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.Rule.cs[12-15]
### Suggested change

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


Grey Divider

Qodo Logo

@Oceania2018 Oceania2018 merged commit 14db7f1 into SciSharp:master May 4, 2026
4 checks passed
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.

3 participants