Skip to content

RG-T117 Chatbox fixes #451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ucswift merged 2 commits into from
Aug 8, 2026
Merged

RG-T117 Chatbox fixes #451

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e07e672
RG-T117 Chatbox fixes
ucswift Aug 8, 2026
02e4fa7
RG-T117 PR#451 fixes
ucswift Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions Core/Resgrid.Chatbot/Services/ChatbotDepartmentConfigService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ public async Task<ChatbotDepartmentConfig> SaveConfigAsync(ChatbotDepartmentConf
else
config.LlmApiKey = _encryptionService.Encrypt(newPlaintextLlmKey);

ValidateColumnLengths(config);

if (existing == null)
{
config.Id = Guid.NewGuid().ToString("N");
Expand Down Expand Up @@ -151,6 +153,26 @@ public async Task InvalidateCacheAsync(int departmentId)
}
}

/// <summary>
/// Guards against SQL truncation (error 8152) by validating string fields against the
/// ChatbotDepartmentConfigs column sizes (M0068/M0070) before hitting the database.
/// LlmApiKey is checked post-encryption since the ciphertext is what gets stored.
/// </summary>
private static void ValidateColumnLengths(ChatbotDepartmentConfig config)
{
if (config.AllowedPlatforms != null && config.AllowedPlatforms.Length > 500)

@Resgrid-Bot Resgrid-Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules low

Magic number obscures the inline column-size limit of 500, violating Rule [9] by breaking explicit schema contracts. Extract this literal to a private const (e.g., MaxAllowedPlatformsLength) for the length check and error message, and apply the same fix to CallsController.cs:1710 and lines 166, 169, and 172 of ChatbotDepartmentConfigService.cs.

Kody rule violation: Replace magic numbers with named constants

Prompt for LLM 

File Core/Resgrid.Chatbot/Services/ChatbotDepartmentConfigService.cs:

Line 163:

Magic number obscures the inline column-size limit of 500, violating Rule [9] by breaking explicit schema contracts. Extract this literal to a private const (e.g., `MaxAllowedPlatformsLength`) for the length check and error message, and apply the same fix to `CallsController.cs:1710` and lines 166, 169, and 172 of `ChatbotDepartmentConfigService.cs`.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

throw new ArgumentException("AllowedPlatforms cannot exceed 500 characters.", nameof(config));

if (config.LlmApiEndpoint != null && config.LlmApiEndpoint.Length > 500)
throw new ArgumentException("LlmApiEndpoint cannot exceed 500 characters.", nameof(config));

if (config.LlmModelName != null && config.LlmModelName.Length > 200)
throw new ArgumentException("LlmModelName cannot exceed 200 characters.", nameof(config));

if (config.LlmApiKey != null && config.LlmApiKey.Length > 1000)
throw new ArgumentException("Encrypted LlmApiKey cannot exceed 1000 characters; supply a shorter API key.", nameof(config));

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Map configuration validation failures to HTTP 400.

ValidateColumnLengths throws ArgumentException for request data. Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs catches every exception and returns HTTP 500. An oversized AllowedPlatforms, LlmApiEndpoint, LlmModelName, or API key is therefore reported as a server failure instead of a client validation error.

Use a typed validation exception or a shared validation result. Map it to BadRequest before the generic exception handler.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Core/Resgrid.Chatbot/Services/ChatbotDepartmentConfigService.cs` around lines
156 - 173, Update ValidateColumnLengths and the ChatbotController exception
handling so oversized configuration fields produce a typed validation failure
that is mapped to BadRequest (HTTP 400) before the generic exception handler.
Preserve the existing HTTP 500 behavior for unrelated exceptions and apply the
validation path to AllowedPlatforms, LlmApiEndpoint, LlmModelName, and encrypted
LlmApiKey.

}

private static string CacheKey(int departmentId) => $"ChatbotDeptConfig_{departmentId}";
}
}
10 changes: 10 additions & 0 deletions Web/Resgrid.Web.Services/Controllers/v4/CallsController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1701,9 +1701,19 @@ public async Task<ActionResult<CallHistoryResult>> GetCallHistory(int callId)
/// <returns>Array of CallResult objects for each call in the department within the range</returns>
[HttpGet("GetCalls")]
[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
[Authorize(Policy = ResgridResources.Call_View)]
public async Task<ActionResult<ActiveCallsResult>> GetCalls(DateTime startDate, DateTime endDate)
{
// Missing query params bind to DateTime.MinValue (0001-01-01), which is below the SQL
// Server datetime floor (1753-01-01) and throws SqlDateTime overflow at the repository.
var sqlMinDate = new DateTime(1753, 1, 1);
if (startDate < sqlMinDate || endDate < sqlMinDate)
return BadRequest("startDate and endDate are required and must be valid dates (on or after 1753-01-01).");

if (endDate < startDate)
return BadRequest("endDate must be on or after startDate.");

var result = new ActiveCallsResult();

var calls = (await _callsService.GetAllCallsByDepartmentDateRangeAsync(DepartmentId, startDate, endDate)).OrderByDescending(x => x.LoggedOn);
Expand Down
8 changes: 8 additions & 0 deletions Web/Resgrid.Web/Areas/User/Controllers/ChatbotSettingsController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ public async Task<IActionResult> Index(ChatbotSettingsModel model, CancellationT
if (!await _authorizationService.CanUserModifyDepartmentAsync(UserId, DepartmentId))
return Unauthorized();

if (!ModelState.IsValid)
{
var existing = await _chatbotConfigService.GetConfigAsync(DepartmentId);

@Resgrid-Bot Resgrid-Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules high

Unguarded async operation occurs because the awaited call to GetConfigAsync on line 60 falls outside the try/catch block, violating Rule [1]. Wrap this await in error handling or move the ModelState branch inside the existing try block to log and handle exceptions with context.

Kody rule violation: Handle async operations with proper error handling

Prompt for LLM 

File Web/Resgrid.Web/Areas/User/Controllers/ChatbotSettingsController.cs:

Line 60:

Unguarded async operation occurs because the awaited call to `GetConfigAsync` on line 60 falls outside the try/catch block, violating Rule [1]. Wrap this await in error handling or move the `ModelState` branch inside the existing try block to log and handle exceptions with context.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

@Resgrid-Bot Resgrid-Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules high

Unguarded external call on line 60 leaves GetConfigAsync exposed to unhandled exceptions, violating Rule [27]. Enclose this call in a try/catch block to log contextual data (DepartmentId) and map the error to a fallback view or application-level exception.

Kody rule violation: Add try-catch blocks for external calls

Prompt for LLM 

File Web/Resgrid.Web/Areas/User/Controllers/ChatbotSettingsController.cs:

Line 60:

Unguarded external call on line 60 leaves `GetConfigAsync` exposed to unhandled exceptions, violating Rule [27]. Enclose this call in a try/catch block to log contextual data (`DepartmentId`) and map the error to a fallback view or application-level exception.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

model.HasLlmApiKey = !string.IsNullOrWhiteSpace(existing?.LlmApiKey);
model.LlmApiKey = null;
return View(model);
}

try
{
var config = new ChatbotDepartmentConfig
Expand Down
12 changes: 11 additions & 1 deletion Web/Resgrid.Web/Areas/User/Models/ChatbotSettingsModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.ComponentModel.DataAnnotations;

namespace Resgrid.Web.Areas.User.Models
{
public class ChatbotSettingsModel : BaseUserModel
Expand All @@ -8,6 +10,7 @@ public class ChatbotSettingsModel : BaseUserModel
public bool IsEnabled { get; set; }

/// <summary>Comma-separated platform names allowed for this department, or "*" for all.</summary>
[StringLength(500, ErrorMessage = "Allowed platforms cannot exceed 500 characters.")]
public string AllowedPlatforms { get; set; } = "*";

public bool AllowDispatchViaChatbot { get; set; }
Expand All @@ -24,10 +27,17 @@ public class ChatbotSettingsModel : BaseUserModel

// Department's own LLM/AI provider (optional). When set, the chatbot keeps this department's
// processing with their provider instead of the Resgrid system LLM.
[StringLength(500, ErrorMessage = "API endpoint cannot exceed 500 characters.")]
public string LlmApiEndpoint { get; set; }
Comment thread
coderabbitai[bot] marked this conversation as resolved.

[StringLength(200, ErrorMessage = "Model name cannot exceed 200 characters.")]
public string LlmModelName { get; set; }

/// <summary>Write-only: a new API key to store. Never populated on read (see HasLlmApiKey).</summary>
/// <summary>
/// Write-only: a new API key to store. Never populated on read (see HasLlmApiKey).
/// Cap is 700 so the AES+base64 ciphertext fits the 1000-char LlmApiKey column.
/// </summary>
[StringLength(700, ErrorMessage = "API key cannot exceed 700 characters.")]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
public string LlmApiKey { get; set; }

/// <summary>True when an LLM API key is already stored (so the UI can indicate it without exposing it).</summary>
Expand Down
12 changes: 8 additions & 4 deletions Web/Resgrid.Web/Areas/User/Views/ChatbotSettings/Index.cshtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
<div class="form-group">
<label class="col-sm-3 control-label">@localizer["ChatbotAllowedPlatforms"]</label>
<div class="col-sm-9">
<input type="text" class="form-control" asp-for="AllowedPlatforms" placeholder="*">
<input type="text" class="form-control" asp-for="AllowedPlatforms" placeholder="*" maxlength="500">
<span asp-validation-for="AllowedPlatforms" class="text-danger"></span>
<span class="help-block m-b-none">@localizer["ChatbotAllowedPlatformsHelp"]</span>
</div>
</div>
Expand Down Expand Up @@ -118,21 +119,24 @@
<div class="form-group">
<label class="col-sm-3 control-label">@localizer["ChatbotLlmEndpoint"]</label>
<div class="col-sm-9">
<input type="text" class="form-control" asp-for="LlmApiEndpoint" placeholder="https://api.your-provider.com/v1/chat/completions">
<input type="text" class="form-control" asp-for="LlmApiEndpoint" placeholder="https://api.your-provider.com/v1/chat/completions" maxlength="500">
<span asp-validation-for="LlmApiEndpoint" class="text-danger"></span>
</div>
</div>

<div class="form-group">
<label class="col-sm-3 control-label">@localizer["ChatbotLlmModel"]</label>
<div class="col-sm-9">
<input type="text" class="form-control" asp-for="LlmModelName" placeholder="gpt-4o, deepseek-chat, ...">
<input type="text" class="form-control" asp-for="LlmModelName" placeholder="gpt-4o, deepseek-chat, ..." maxlength="200">
<span asp-validation-for="LlmModelName" class="text-danger"></span>
</div>
</div>

<div class="form-group">
<label class="col-sm-3 control-label">@localizer["ChatbotLlmApiKey"]</label>
<div class="col-sm-9">
<input type="password" class="form-control" asp-for="LlmApiKey" autocomplete="new-password">
<input type="password" class="form-control" asp-for="LlmApiKey" autocomplete="new-password" maxlength="700">
<span asp-validation-for="LlmApiKey" class="text-danger"></span>
<span class="help-block m-b-none">
@if (Model.HasLlmApiKey)
{
Expand Down
Loading