Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Important Review skippedToo many files! This PR contains 153 files, which is 53 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (15)
📒 Files selected for processing (153)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment has been minimized.
This comment has been minimized.
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
| var request = new HttpRequestMessage(HttpMethod.Post, endpoint) | ||
| { | ||
| Content = new StringContent(JsonConvert.SerializeObject(requestBody), Encoding.UTF8, "application/json") | ||
| }; |
There was a problem hiding this comment.
WHAT: HttpRequestMessage implements IDisposable but is never disposed (no using). WHY: Undisposed request messages leak unmanaged resources and socket buffers, especially under load. HOW: Wrap in using var request = new HttpRequestMessage(...) (or a using block) so it is deterministically disposed after SendAsync.
Kody rule violation: Use using statements for disposable resources
Prompt for LLM
File Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:
Line 77 to 80:
WHAT: HttpRequestMessage implements IDisposable but is never disposed (no using). WHY: Undisposed request messages leak unmanaged resources and socket buffers, especially under load. HOW: Wrap in `using var request = new HttpRequestMessage(...)` (or a using block) so it is deterministically disposed after SendAsync.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logging.LogException(ex); |
There was a problem hiding this comment.
WHAT: The catch block logs only the raw exception without any operation context or identifiers. WHY: Rule 3 requires error logs to include the operation name and relevant identifiers (e.g., userId, text snippet) as structured fields so failures are traceable and diagnosable. Logging just ex is analogous to console.error(e) — it loses the context of which user or operation failed. HOW: Pass contextual fields alongside the exception, e.g. Logging.LogException(ex, $"PushToUserAsync failed for userId={userId}"); or use a structured logging overload that accepts a message template with operation name and userId parameters.
Also found in:
Core/Resgrid.Services/IncidentCommandService.cs:1486-1486Web/Resgrid.Web/Areas/User/Apps/src/components/chat/useChatBootstrap.ts:29-29Core/Resgrid.Services/ChatProvisioningEventService.cs:42-42Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:108-108Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/SettingsTab.tsx:56-56Core/Resgrid.Services/IncidentCommandService.cs:1516-1516Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs:383-383Web/Resgrid.Web.Eventing/Worker.cs:226-226Workers/Resgrid.Workers.Framework/Logic/ChatRetentionLogic.cs:82-82Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ThreadPanel.tsx:29-29Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:62-62Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:125-125Core/Resgrid.Services/PushService.cs:247-247Core/Resgrid.Services/ChatProvisioningEventService.cs:89-89Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsx:58-58Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:106-106Core/Resgrid.Services/ChatMessageService.cs:183-183Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:59-59Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ExportsTab.tsx:35-35Core/Resgrid.Services/PushService.cs:260-260Core/Resgrid.Services/ChatProvisioningEventService.cs:58-58Providers/Resgrid.Providers.Messaging/GifProvider.cs:53-53Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:198-198Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs:408-408Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:105-105Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatbotElement.tsx:54-54Providers/Resgrid.Providers.Messaging/GifProvider.cs:72-72Core/Resgrid.Services/ChatProvisioningEventService.cs:105-105Core/Resgrid.Services/PushService.cs:277-277Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatbotElement.tsx:73-73Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatbotElement.tsx:82-82Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:129-129Web/Resgrid.Web/Areas/User/Apps/src/components/chat/MembersPanel.tsx:48-48Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:118-118Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:153-153Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1150-1150Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:161-161Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1197-1197Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:775-775Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1504-1504Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1549-1549Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1447-1447Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:381-381Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:669-669Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:138-138Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1291-1291Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:456-456Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1103-1103Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1336-1336Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:940-940Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:170-170Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:119-119Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:238-238Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1246-1246Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:273-273Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:612-612Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:254-254Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:581-581Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:895-895Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1005-1005Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:700-700Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1621-1621Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:216-216Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:336-336Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:743-743Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1058-1058Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1367-1367Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:210-210Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:549-549Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:805-805Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:179-179Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:306-306Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:487-487Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:518-518Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:411-411Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:89-89Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1399-1399Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:864-864Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1591-1591Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:242-242Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:149-149
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Core/Resgrid.Chatbot/Services/ChatWebChatNotifier.cs:
Line 53:
WHAT: The catch block logs only the raw exception without any operation context or identifiers. WHY: Rule 3 requires error logs to include the operation name and relevant identifiers (e.g., userId, text snippet) as structured fields so failures are traceable and diagnosable. Logging just `ex` is analogous to `console.error(e)` — it loses the context of which user or operation failed. HOW: Pass contextual fields alongside the exception, e.g. `Logging.LogException(ex, $"PushToUserAsync failed for userId={userId}");` or use a structured logging overload that accepts a message template with operation name and userId parameters.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // Unknown intent: try the guard-railed conversational LLM fallback before giving up. | ||
| // Operational commands never reach here — matched intents dispatched above — so the | ||
| // fallback can only produce informational replies. | ||
| var fallbackResponse = await _conversationalFallback.TryHandleAsync(message, session); |
There was a problem hiding this comment.
WHAT: The new conversational LLM fallback is awaited without any try/catch. WHY: TryHandleAsync is an external/network dependency (an LLM service); an exception, timeout, or transient failure would propagate and crash the entire ProcessMessageAsync flow, losing the user's message. HOW: Wrap the call in try/catch, log structured context (intent, session id, message id), and fall through to the existing "I didn't understand" response on failure rather than letting the exception escape.
Also found in:
Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs:603-603Providers/Resgrid.Providers.Bus.Rabbit/RabbitTopicProvider.cs:123-123Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs:318-318Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:134-134Core/Resgrid.Services/ChatChannelService.cs:179-179Web/Resgrid.Web/Areas/User/Apps/src/components/chat/PinsPanel.tsx:48-48Core/Resgrid.Services/ChatChannelService.cs:207-207Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatApi.ts:50-50Core/Resgrid.Services/ChatChannelService.cs:335-335Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatApi.ts:257-257Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatModerationApi.ts:146-146Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ThreadPanel.tsx:41-41Web/Resgrid.Web.Services/Hubs/ChatHub.cs:45-45Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:100-100Core/Resgrid.Services/ChatPresenceService.cs:27-27Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:385-385Web/Resgrid.Web/Areas/User/Apps/src/components/chat/MembersPanel.tsx:22-22Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:120-120Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:503-503Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:33-33Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:160-160Web/Resgrid.Web.Services/Hubs/ChatHub.cs:57-57Core/Resgrid.Services/PushService.cs:234-234Core/Resgrid.Services/PushService.cs:239-239Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:45-45Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:101-101Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:429-429Core/Resgrid.Services/ChatPresenceService.cs:41-41Web/Resgrid.Web.Services/Hubs/ChatHub.cs:61-61Web/Resgrid.Web.Services/Hubs/ChatHub.cs:109-109Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:262-262Web/Resgrid.Web.Services/Hubs/ChatHub.cs:133-133Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:228-228Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:194-194Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:48-48Web/Resgrid.Web.Services/Hubs/ChatHub.cs:124-124Core/Resgrid.Services/ChatPresenceService.cs:29-29Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:57-57Web/Resgrid.Web.Services/Hubs/ChatHub.cs:79-79Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:133-133Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:61-61Core/Resgrid.Services/ChatPresenceService.cs:36-36Web/Resgrid.Web.Services/Hubs/ChatHub.cs:102-102Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:109-109Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:124-124Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:84-84Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:106-106Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:121-121Web/Resgrid.Web.Services/Hubs/ChatHub.cs:84-84Web/Resgrid.Web.Services/Hubs/ChatHub.cs:106-106Web/Resgrid.Web.Services/Hubs/ChatHub.cs:121-121Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:79-79Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:102-102Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:117-117Web/Resgrid.Web.Services/Hubs/ChatHub.cs:117-117
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs:
Line 558:
WHAT: The new conversational LLM fallback is awaited without any try/catch. WHY: TryHandleAsync is an external/network dependency (an LLM service); an exception, timeout, or transient failure would propagate and crash the entire ProcessMessageAsync flow, losing the user's message. HOW: Wrap the call in try/catch, log structured context (intent, session id, message id), and fall through to the existing "I didn't understand" response on failure rather than letting the exception escape.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public object IdValue | ||
| { | ||
| get { return ChatChannelId; } | ||
| set { ChatChannelId = (string)value; } |
There was a problem hiding this comment.
Violates team rule 'Use safe type casting with as operator': Use the as operator or pattern matching for safe casts and guard null results before usage.
Also found in:
Core/Resgrid.Model/Chat/ChatChannel.cs:133-133Core/Resgrid.Model/Chat/ChatChannel.cs:209-209Core/Resgrid.Model/Chat/ChatInteractions.cs:45-45Core/Resgrid.Model/Chat/ChatInteractions.cs:88-88Core/Resgrid.Model/Chat/ChatInteractions.cs:133-133Core/Resgrid.Model/Chat/ChatMessage.cs:90-90Core/Resgrid.Model/Chat/ChatMessage.cs:131-131Core/Resgrid.Model/Chat/ChatMessage.cs:186-186Core/Resgrid.Model/Chat/ChatModeration.cs:51-51Core/Resgrid.Model/Chat/ChatModeration.cs:101-101Core/Resgrid.Model/Chat/ChatModeration.cs:147-147Core/Resgrid.Model/Chat/ChatModeration.cs:199-199Core/Resgrid.Model/Events/ChatEvents.cs:5-5Core/Resgrid.Model/Services/IChatServices.cs:130-130Core/Resgrid.Services/ChatChannelService.cs:96-96Core/Resgrid.Services/ChatChannelService.cs:254-254Core/Resgrid.Services/ChatChannelService.cs:550-550Core/Resgrid.Services/ChatMessageService.cs:342-342Core/Resgrid.Services/ChatModerationService.cs:155-155Core/Resgrid.Services/ChatModerationService.cs:177-177Providers/Resgrid.Providers.Messaging/GifProvider.cs:85-85Providers/Resgrid.Providers.Messaging/GifProvider.cs:86-86Providers/Resgrid.Providers.Messaging/GifProvider.cs:87-87Providers/Resgrid.Providers.Messaging/GifProvider.cs:88-88Providers/Resgrid.Providers.Messaging/GifProvider.cs:110-110Providers/Resgrid.Providers.Messaging/GifProvider.cs:111-111Providers/Resgrid.Providers.Messaging/GifProvider.cs:112-112Providers/Resgrid.Providers.Messaging/GifProvider.cs:113-113Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:56-56Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:62-62Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:65-65Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:285-285Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:106-106Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:155-155Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:156-156Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:224-224Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:237-237Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:255-255Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:306-306Tests/Resgrid.Tests/Services/ChatPermissionServiceTests.cs:59-59Tests/Resgrid.Tests/Services/ChatPermissionServiceTests.cs:170-170Tests/Resgrid.Tests/Services/ChatPermissionServiceTests.cs:296-296Tests/Resgrid.Tests/Services/ChatPermissionServiceTests.cs:768-768Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:709-709Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:710-710Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1350-1350Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:74-74Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:120-120Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:429-429Workers/Resgrid.Workers.Console/Tasks/ChatExportTask.cs:39-39Workers/Resgrid.Workers.Console/Tasks/ChatRetentionTask.cs:39-39Workers/Resgrid.Workers.Framework/Logic/ChatExportLogic.cs:70-70Workers/Resgrid.Workers.Framework/Logic/ChatRetentionLogic.cs:76-76Workers/Resgrid.Workers.Framework/Logic/ChatbotMessageLogic.cs:55-55
Prompt for LLM
File Core/Resgrid.Model/Chat/ChatChannel.cs:
Line 87:
Violates team rule 'Use safe type casting with as operator': Use the as operator or pattern matching for safe casts and guard null results before usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public string PayloadJson { get; set; } | ||
|
|
||
| /// <summary>Target a single user's devices instead of the channel group (chatbot, DM invites, badges).</summary> | ||
| public string TargetUserId { get; set; } |
There was a problem hiding this comment.
WHAT: The auto-property TargetUserId has no default value. WHY: Rule 32 requires properties to be initialized with sensible defaults; a bare string property defaults to null, which could be mistaken for a valid 'no target' state. HOW: Initialize with string.Empty (e.g., public string TargetUserId { get; set; } = string.Empty;) or explicitly mark as nullable to make intent clear.
Also found in:
Workers/Resgrid.Workers.Console/Commands/ChatExportCommand.cs:16-16Core/Resgrid.Model/Events/ChatEvents.cs:15-15Core/Resgrid.Model/Events/ChatEvents.cs:12-12Core/Resgrid.Model/Events/ChatEvents.cs:21-21Workers/Resgrid.Workers.Console/Commands/ChatRetentionCommand.cs:16-16Core/Resgrid.Model/Chat/ChatInteractions.cs:11-11Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs:428-428Core/Resgrid.Model/Chat/ChatModeration.cs:11-11Core/Resgrid.Model/Chat/ChatChannel.cs:16-16Core/Resgrid.Model/Chat/ChatMessage.cs:15-15Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs:427-427Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:35-35Core/Resgrid.Chatbot/Interfaces/IChatCompletionClient.cs:9-9Core/Resgrid.Model/Providers/IGifProvider.cs:9-9Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:948-948Core/Resgrid.Model/Chat/ChatMessage.cs:155-155Core/Resgrid.Model/Services/IChatServices.cs:204-204Core/Resgrid.Chatbot/Interfaces/IChatCompletionClient.cs:10-10Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:969-969Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1089-1089Core/Resgrid.Model/Chat/ChatModeration.cs:35-35Core/Resgrid.Model/Chat/ChatModeration.cs:85-85Core/Resgrid.Model/Chat/ChatModeration.cs:83-83Core/Resgrid.Model/Chat/ChatModeration.cs:24-24Core/Resgrid.Model/Chat/ChatModeration.cs:183-183Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1022-1022Core/Resgrid.Model/Services/IChatServices.cs:183-183Core/Resgrid.Model/Services/IChatServices.cs:185-185Core/Resgrid.Model/Services/IChatServices.cs:190-190Core/Resgrid.Model/Services/IChatServices.cs:193-193Core/Resgrid.Model/Services/IChatServices.cs:196-196Core/Resgrid.Model/Services/IChatServices.cs:198-198Core/Resgrid.Model/Services/IChatServices.cs:200-200Core/Resgrid.Model/Chat/ChatModeration.cs:31-31Core/Resgrid.Model/Chat/ChatChannel.cs:25-25Core/Resgrid.Model/Chat/ChatMessage.cs:37-37Core/Resgrid.Model/Chat/ChatMessage.cs:108-108Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:65-65Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:152-152Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:220-220Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:231-231Core/Resgrid.Model/Chat/ChatModeration.cs:64-64Core/Resgrid.Model/Chat/ChatModeration.cs:111-111Core/Resgrid.Model/Chat/ChatModeration.cs:157-157Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:253-253Core/Resgrid.Model/Providers/IGifProvider.cs:12-12Core/Resgrid.Model/Chat/ChatMessage.cs:100-100Core/Resgrid.Model/Chat/ChatMessage.cs:145-145Core/Resgrid.Model/Chat/ChatInteractions.cs:13-13Core/Resgrid.Model/Chat/ChatInteractions.cs:15-15Core/Resgrid.Model/Chat/ChatInteractions.cs:57-57Core/Resgrid.Model/Chat/ChatInteractions.cs:59-59Core/Resgrid.Model/Chat/ChatInteractions.cs:104-104Core/Resgrid.Model/Chat/ChatInteractions.cs:106-106Core/Resgrid.Model/Chat/ChatMessage.cs:168-168Core/Resgrid.Model/Chat/ChatInteractions.cs:66-66Core/Resgrid.Model/Chat/ChatMessage.cs:153-153Core/Resgrid.Model/Providers/IGifProvider.cs:14-14Core/Resgrid.Model/Providers/IGifProvider.cs:10-10Core/Resgrid.Model/Chat/ChatMessage.cs:113-113Core/Resgrid.Model/Chat/ChatModeration.cs:13-13Core/Resgrid.Model/Chat/ChatModeration.cs:15-15Core/Resgrid.Model/Chat/ChatModeration.cs:19-19Core/Resgrid.Model/Chat/ChatModeration.cs:68-68Core/Resgrid.Model/Chat/ChatModeration.cs:70-70Core/Resgrid.Model/Chat/ChatModeration.cs:72-72Core/Resgrid.Model/Chat/ChatModeration.cs:79-79Core/Resgrid.Model/Chat/ChatModeration.cs:161-161Core/Resgrid.Model/Chat/ChatModeration.cs:166-166Core/Resgrid.Model/Chat/ChatChannel.cs:23-23Core/Resgrid.Model/Chat/ChatChannel.cs:27-27Core/Resgrid.Model/Chat/ChatChannel.cs:38-38Core/Resgrid.Model/Chat/ChatChannel.cs:41-41Core/Resgrid.Model/Chat/ChatChannel.cs:44-44Core/Resgrid.Model/Chat/ChatChannel.cs:50-50Core/Resgrid.Model/Chat/ChatChannel.cs:59-59Core/Resgrid.Model/Chat/ChatChannel.cs:100-100Core/Resgrid.Model/Chat/ChatChannel.cs:102-102Core/Resgrid.Model/Chat/ChatChannel.cs:113-113Core/Resgrid.Model/Chat/ChatChannel.cs:115-115Core/Resgrid.Model/Chat/ChatChannel.cs:148-148Core/Resgrid.Model/Chat/ChatChannel.cs:150-150Core/Resgrid.Model/Chat/ChatChannel.cs:157-157Core/Resgrid.Model/Chat/ChatChannel.cs:162-162Core/Resgrid.Model/Chat/ChatChannel.cs:168-168Core/Resgrid.Model/Chat/ChatChannel.cs:188-188Core/Resgrid.Model/Chat/ChatInteractions.cs:22-22Core/Resgrid.Model/Chat/ChatInteractions.cs:110-110Core/Resgrid.Model/Chat/ChatInteractions.cs:27-27Core/Resgrid.Model/Chat/ChatInteractions.cs:55-55Core/Resgrid.Model/Chat/ChatMessage.cs:17-17Core/Resgrid.Model/Chat/ChatInteractions.cs:102-102Core/Resgrid.Model/Chat/ChatMessage.cs:102-102Core/Resgrid.Model/Chat/ChatMessage.cs:104-104Core/Resgrid.Model/Chat/ChatMessage.cs:147-147Core/Resgrid.Model/Chat/ChatMessage.cs:149-149Core/Resgrid.Model/Chat/ChatMessage.cs:160-160Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:268-268Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:278-278Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:283-283Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:298-298Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:303-303Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:354-354Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:359-359Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:379-379Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:389-389Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:394-394Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:409-409Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:429-429Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:434-434Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:454-454Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:464-464Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:494-494Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:499-499Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:504-504Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:520-520Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:530-530Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:546-546Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:551-551Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:561-561Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:571-571Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:627-627Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:632-632Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:637-637Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:642-642Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:668-668Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:673-673Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:678-678Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:683-683Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:693-693Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:708-708Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:718-718Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:729-729Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:734-734Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:739-739Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:744-744Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:759-759Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:769-769Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:774-774Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:785-785Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:831-831Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:836-836Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:841-841Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:876-876Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:887-887Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:892-892Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:897-897Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:902-902Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:927-927Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:943-943Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:959-959Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:964-964Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:995-995Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1006-1006Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1011-1011Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1044-1044Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1049-1049Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1074-1074Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1084-1084Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1105-1105Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1131-1131Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1142-1142Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1174-1174Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1190-1190Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1201-1201Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1217-1217Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1238-1238Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:1290-1290
Kody rule violation: Initialize properties with default values
Prompt for LLM
File Core/Resgrid.Model/Events/ChatEvents.cs:
Line 24:
WHAT: The auto-property `TargetUserId` has no default value. WHY: Rule 32 requires properties to be initialized with sensible defaults; a bare string property defaults to null, which could be mistaken for a valid 'no target' state. HOW: Initialize with `string.Empty` (e.g., `public string TargetUserId { get; set; } = string.Empty;`) or explicitly mark as nullable to make intent clear.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// flags) — the retention purge. Moderation action rows are audit and are NOT touched. Returns | ||
| /// the number of messages removed. | ||
| /// </summary> | ||
| Task<int> DeleteMessagesByIdsAsync(List<string> chatMessageIds, CancellationToken cancellationToken); |
There was a problem hiding this comment.
WHAT: The parameter type requires a mutable List in a public interface method. WHY: Forcing callers to supply List is unnecessarily restrictive and inconsistent with sibling methods (e.g., lines 24, 82, 113, 118) that accept IEnumerable; IReadOnlyList or IEnumerable is more appropriate for a read-only input. HOW: Change the parameter type to IReadOnlyList or IEnumerable.
Also found in:
Core/Resgrid.Model/Repositories/IChatRepositories.cs:92-92Core/Resgrid.Services/ChatModerationService.cs:76-76Core/Resgrid.Services/ChatChannelService.cs:55-55Core/Resgrid.Services/ChatChannelService.cs:265-265Core/Resgrid.Model/Providers/IGifProvider.cs:28-28Core/Resgrid.Services/ChatModerationService.cs:190-190Core/Resgrid.Services/ChatChannelService.cs:271-271Core/Resgrid.Services/ChatModerationService.cs:219-219Core/Resgrid.Model/Providers/IGifProvider.cs:31-31Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:900-900Core/Resgrid.Services/ChatMessageService.cs:324-324Core/Resgrid.Services/ChatMessageService.cs:330-330Core/Resgrid.Services/ChatMessageService.cs:376-376Core/Resgrid.Services/ChatMessageService.cs:382-382Core/Resgrid.Services/ChatMessageService.cs:195-195Core/Resgrid.Services/ChatMessageService.cs:201-201Core/Resgrid.Services/ChatMessageService.cs:207-207Core/Resgrid.Services/ChatMessageService.cs:353-353Core/Resgrid.Services/ChatMessageService.cs:414-414
Kody rule violation: Use IReadOnlyList for immutable collections
Prompt for LLM
File Core/Resgrid.Model/Repositories/IChatRepositories.cs:
Line 99:
WHAT: The parameter type requires a mutable List<string> in a public interface method. WHY: Forcing callers to supply List<T> is unnecessarily restrictive and inconsistent with sibling methods (e.g., lines 24, 82, 113, 118) that accept IEnumerable<string>; IReadOnlyList<string> or IEnumerable<string> is more appropriate for a read-only input. HOW: Change the parameter type to IReadOnlyList<string> or IEnumerable<string>.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| catch (Exception) | ||
| { | ||
| // Unique (DepartmentId, DmKey) index backstops the check-then-insert race; adopt the winner. | ||
| var winner = await _chatChannelRepository.GetByDmKeyAsync(departmentId, dmKey); | ||
| if (winner != null) | ||
| return winner; | ||
|
|
||
| throw; | ||
| } |
There was a problem hiding this comment.
WHAT: Catch a specific DB exception type (e.g., DbUpdateException) and check for unique-constraint violation instead of catching all Exceptions. WHY: Catching Exception means non-DB errors (NullReferenceException, timeout, etc.) also trigger the re-fetch logic, masking real bugs. HOW: catch (DbUpdateException ex) when (IsUniqueConstraintViolation(ex)) { ... } — let all other exceptions propagate immediately.
Also found in:
Core/Resgrid.Services/ChatMessageService.cs:294-298Core/Resgrid.Services/ChatChannelService.cs:617-625
Kody rule violation: Implement proper database error checking
Prompt for LLM
File Core/Resgrid.Services/ChatChannelService.cs:
Line 140 to 148:
WHAT: Catch a specific DB exception type (e.g., DbUpdateException) and check for unique-constraint violation instead of catching all Exceptions. WHY: Catching Exception means non-DB errors (NullReferenceException, timeout, etc.) also trigger the re-fetch logic, masking real bugs. HOW: catch (DbUpdateException ex) when (IsUniqueConstraintViolation(ex)) { ... } — let all other exceptions propagate immediately.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var existing = (await _chatChannelRepository.GetByCallIdAsync(callId))? | ||
| .FirstOrDefault(c => c.ChannelType == (int)ChatChannelType.Incident); |
There was a problem hiding this comment.
WHAT: Extract the 'GetByCallIdAsync + filter by channel type' query into a shared repository or service method. WHY: The same query pattern appears again at lines 494-495 with only the channel type differing; duplicating it risks divergence. HOW: Create 'GetChannelByCallIdAndTypeAsync(callId, ChatChannelType type)' in the repository and call it from both EnsureIncidentChannelAsync and EnsureCommandChannelAsync.
Also found in:
Core/Resgrid.Services/ChatChannelService.cs:494-495
Kody rule violation: Extract common query logic
Prompt for LLM
File Core/Resgrid.Services/ChatChannelService.cs:
Line 451 to 452:
WHAT: Extract the 'GetByCallIdAsync + filter by channel type' query into a shared repository or service method. WHY: The same query pattern appears again at lines 494-495 with only the channel type differing; duplicating it risks divergence. HOW: Create 'GetChannelByCallIdAndTypeAsync(callId, ChatChannelType type)' in the repository and call it from both EnsureIncidentChannelAsync and EnsureCommandChannelAsync.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| var senderDisplayName = await ResolveSenderDisplayNameAsync(request, channel); | ||
|
|
||
| var seq = await _chatChannelRepository.AllocateNextMessageSeqAsync(channel.ChatChannelId, DateTime.UtcNow); |
There was a problem hiding this comment.
WHAT: AllocateNextMessageSeqAsync returns the newly-allocated sequence but the in-memory channel object is never updated, so the object passed to NotifyMessageSentAsync still carries the pre-send LastMessageSeq. WHY: ChatNotificationService.NotifyMessageSentAsync computes each recipient's unread count as channel.LastMessageSeq - member.LastReadSeq (ChatNotificationService.cs:91,105); with the stale value this undercounts the badge, and the Math.Max(unread,1) mask means recipients always see 1 instead of their true unread total. HOW: After allocating the sequence, assign it back to the local entity before fanning out, e.g. channel.LastMessageSeq = seq; (and optionally channel.LastMessageOn = message.SentOn;) so the notification service computes the correct count.
var seq = await _chatChannelRepository.AllocateNextMessageSeqAsync(channel.ChatChannelId, DateTime.UtcNow);
channel.LastMessageSeq = seq;
channel.LastMessageOn = message?.SentOn ?? DateTime.UtcNow;Prompt for LLM
File Core/Resgrid.Services/ChatMessageService.cs:
Line 105:
WHAT: AllocateNextMessageSeqAsync returns the newly-allocated sequence but the in-memory `channel` object is never updated, so the object passed to NotifyMessageSentAsync still carries the pre-send LastMessageSeq. WHY: ChatNotificationService.NotifyMessageSentAsync computes each recipient's unread count as `channel.LastMessageSeq - member.LastReadSeq` (ChatNotificationService.cs:91,105); with the stale value this undercounts the badge, and the Math.Max(unread,1) mask means recipients always see 1 instead of their true unread total. HOW: After allocating the sequence, assign it back to the local entity before fanning out, e.g. `channel.LastMessageSeq = seq;` (and optionally `channel.LastMessageOn = message.SentOn;`) so the notification service computes the correct count.
Suggested Code:
var seq = await _chatChannelRepository.AllocateNextMessageSeqAsync(channel.ChatChannelId, DateTime.UtcNow);
channel.LastMessageSeq = seq;
channel.LastMessageOn = message?.SentOn ?? DateTime.UtcNow;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| } | ||
|
|
||
| private async Task ProvisionAcksAsync(ChatChannel channel, ChatMessage message, CancellationToken cancellationToken) |
There was a problem hiding this comment.
WHAT: ProvisionAcksAsync is awaited on the request thread and inserts one ChatMessageAck row per audience member in a sequential foreach. WHY: For an Urgent message sent to a DepartmentDefault channel, ResolveChannelAudienceUserIdsAsync returns the entire department roster, so a single send triggers O(N) sequential INSERT round-trips against the database on the request path, scaling linearly with department size (thousands of members) and materially slowing the send. HOW: Insert the ack rows in a single batched/bulk operation (or offload provisioning off the request path like the push fan-out), rather than one InsertAsync per user.
// Batch-insert all ack rows in one DB round-trip (or enqueue off the request path)
await _chatMessageAckRepository.InsertBatchAsync(audience
.Where(u => !string.Equals(u, message.SenderUserId, StringComparison.OrdinalIgnoreCase))
.Select(u => new ChatMessageAck { ChatMessageAckId = Guid.NewGuid().ToString(), ChatMessageId = message.ChatMessageId, ChatChannelId = message.ChatChannelId, DepartmentId = message.DepartmentId, UserId = u, RequiredOn = message.SentOn })
.ToList(), cancellationToken);Prompt for LLM
File Core/Resgrid.Services/ChatMessageService.cs:
Line 513:
WHAT: ProvisionAcksAsync is awaited on the request thread and inserts one ChatMessageAck row per audience member in a sequential foreach. WHY: For an Urgent message sent to a DepartmentDefault channel, ResolveChannelAudienceUserIdsAsync returns the entire department roster, so a single send triggers O(N) sequential INSERT round-trips against the database on the request path, scaling linearly with department size (thousands of members) and materially slowing the send. HOW: Insert the ack rows in a single batched/bulk operation (or offload provisioning off the request path like the push fan-out), rather than one InsertAsync per user.
Suggested Code:
// Batch-insert all ack rows in one DB round-trip (or enqueue off the request path)
await _chatMessageAckRepository.InsertBatchAsync(audience
.Where(u => !string.Equals(u, message.SenderUserId, StringComparison.OrdinalIgnoreCase))
.Select(u => new ChatMessageAck { ChatMessageAckId = Guid.NewGuid().ToString(), ChatMessageId = message.ChatMessageId, ChatChannelId = message.ChatChannelId, DepartmentId = message.DepartmentId, UserId = u, RequiredOn = message.SentOn })
.ToList(), cancellationToken);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| DetailsJson = detailsJson | ||
| }, cancellationToken); | ||
|
|
||
| await _auditService.SaveAuditLogAsync(new AuditLog |
There was a problem hiding this comment.
WHAT: The AuditLog entry (lines 257–267) omits required security-audit fields: result, trace_id, ip, user_agent, and actor.role. WHY: Tamper-evident audit logs for security-relevant moderation actions must capture full context for SIEM ingestion and forensic analysis. HOW: Extend the AuditLog data payload or model to include result (success/failure), trace_id, ip, user_agent, and the actor's role before saving; ensure the log is stored in WORM/immutable storage and signed.
Kody rule violation: Emit tamper-evident audit logs with required fields
Prompt for LLM
File Core/Resgrid.Services/ChatModerationService.cs:
Line 257:
WHAT: The AuditLog entry (lines 257–267) omits required security-audit fields: result, trace_id, ip, user_agent, and actor.role. WHY: Tamper-evident audit logs for security-relevant moderation actions must capture full context for SIEM ingestion and forensic analysis. HOW: Extend the AuditLog data payload or model to include result (success/failure), trace_id, ip, user_agent, and the actor's role before saving; ensure the log is stored in WORM/immutable storage and signed.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Status = (int)ChatFlagStatus.Open | ||
| }, cancellationToken); | ||
|
|
||
| PublishModerationEvent(message.DepartmentId, message.ChatChannelId, new { Type = "flagged", flag.ChatMessageFlagId, chatMessageId }); |
There was a problem hiding this comment.
WHAT: The event type "flagged" is a hardcoded magic string inconsistent with line 269 which uses actionType.ToString(). WHY: Magic strings are error-prone and make it easy to introduce typos that silently break event handling. HOW: Replace with an enum or constant such as ChatModerationActionType.FlagMessage.ToString() or a dedicated ChatEventTypes enum member.
Also found in:
Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatApi.ts:334-334Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:209-209Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:65-65Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsx:42-42Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsx:41-41Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsx:43-43Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatModerationElement.tsx:40-40Core/Resgrid.Services/ChatPermissionService.cs:57-57Core/Resgrid.Services/ChatPermissionService.cs:59-59Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsx:116-116Providers/Resgrid.Providers.Messaging/GifProvider.cs:29-29Providers/Resgrid.Providers.Messaging/GifProvider.cs:32-32Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsx:114-114Core/Resgrid.Services/ChatMessageService.cs:399-399Core/Resgrid.Services/ChatMessageService.cs:369-369Providers/Resgrid.Providers.Messaging/GifProvider.cs:46-46Providers/Resgrid.Providers.Messaging/GifProvider.cs:65-65Core/Resgrid.Services/ChatPermissionService.cs:125-125Core/Resgrid.Services/ChatPermissionService.cs:64-64Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:47-47Core/Resgrid.Services/ChatPermissionService.cs:118-118Core/Resgrid.Services/ChatPermissionService.cs:120-120Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:101-101Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsx:115-115Core/Resgrid.Services/ChatPresenceService.cs:29-29Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:66-66Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:49-49Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:87-87Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:119-119Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:139-139Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:146-146Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:149-149Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:116-116Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:130-130Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:167-167Core/Resgrid.Services/ChatPresenceService.cs:36-36Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:127-127Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:133-133Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:143-143Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:124-124Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:136-136Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:181-181Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:152-152Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:156-156Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:226-226Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:191-191Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:252-252Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:236-236Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:267-267Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:261-261
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File Core/Resgrid.Services/ChatModerationService.cs:
Line 71:
WHAT: The event type "flagged" is a hardcoded magic string inconsistent with line 269 which uses actionType.ToString(). WHY: Magic strings are error-prone and make it easy to introduce typos that silently break event handling. HOW: Replace with an enum or constant such as ChatModerationActionType.FlagMessage.ToString() or a dedicated ChatEventTypes enum member.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (nodes != null) | ||
| { | ||
| foreach (var node in nodes) | ||
| await chatChannelService.EnsureLaneChannelAsync(node); |
There was a problem hiding this comment.
WHAT: An async network/DB call (EnsureLaneChannelAsync) is invoked once per node inside a foreach loop. WHY: Rule [104] flags N+1-style sequential awaits inside loops because they serialize I/O and scale poorly with node count. HOW: Provide a batch API on IChatChannelService (e.g., EnsureLaneChannelsAsync(IEnumerable nodes)) that provisions all lanes in one round-trip, or use Task.WhenAll(nodes.Select(n => chatChannelService.EnsureLaneChannelAsync(n))) if the calls are independent and the service is concurrency-safe.
Also found in:
Core/Resgrid.Services/ChatPermissionService.cs:347-347Core/Resgrid.Services/ChatNotificationService.cs:93-93Core/Resgrid.Services/ChatPermissionService.cs:563-563Core/Resgrid.Services/ChatPermissionService.cs:587-587Core/Resgrid.Services/ChatChannelService.cs:186-186Core/Resgrid.Services/ChatPresenceService.cs:51-55Core/Resgrid.Services/ChatPermissionService.cs:476-476Core/Resgrid.Services/ChatPermissionService.cs:557-557Core/Resgrid.Services/ChatChannelService.cs:221-221Core/Resgrid.Services/ChatChannelService.cs:283-283Core/Resgrid.Services/ChatPermissionService.cs:378-378Core/Resgrid.Services/ChatPermissionService.cs:548-548Core/Resgrid.Services/ChatPermissionService.cs:507-507Core/Resgrid.Services/ChatNotificationService.cs:107-107Core/Resgrid.Services/ChatChannelService.cs:347-347Core/Resgrid.Services/ChatChannelService.cs:292-292Core/Resgrid.Services/ChatPermissionService.cs:497-497Core/Resgrid.Services/ChatPermissionService.cs:537-537Core/Resgrid.Services/ChatChannelService.cs:298-298Core/Resgrid.Services/ChatMessageService.cs:509-509Core/Resgrid.Services/ChatMessageService.cs:519-519Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:103-103Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:194-194Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:193-193
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File Core/Resgrid.Services/ChatProvisioningEventService.cs:
Line 84:
WHAT: An async network/DB call (EnsureLaneChannelAsync) is invoked once per node inside a foreach loop. WHY: Rule [104] flags N+1-style sequential awaits inside loops because they serialize I/O and scale poorly with node count. HOW: Provide a batch API on IChatChannelService (e.g., EnsureLaneChannelsAsync(IEnumerable<Node> nodes)) that provisions all lanes in one round-trip, or use Task.WhenAll(nodes.Select(n => chatChannelService.EnsureLaneChannelAsync(n))) if the calls are independent and the service is concurrency-safe.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| _eventAggregator.AddListener(incidentReopenedHandler); | ||
| } | ||
|
|
||
| private Action<CallAddedEvent> callAddedHandler = async delegate (CallAddedEvent message) |
There was a problem hiding this comment.
WHAT: The field callAddedHandler is assigned only at declaration and never reassigned, but is not marked readonly. WHY: Rule [108] requires immutable instance fields to be marked readonly so the compiler enforces immutability and intent is clear. HOW: Change the declaration to 'private readonly Action callAddedHandler = async delegate (CallAddedEvent message) { ... };'.
Also found in:
Core/Resgrid.Services/ChatProvisioningEventService.cs:62-62Core/Resgrid.Services/ChatProvisioningEventService.cs:93-93Core/Resgrid.Services/ChatProvisioningEventService.cs:46-46
Kody rule violation: Use `readonly` or `const` for Immutable Data
Prompt for LLM
File Core/Resgrid.Services/ChatProvisioningEventService.cs:
Line 30:
WHAT: The field callAddedHandler is assigned only at declaration and never reassigned, but is not marked readonly. WHY: Rule [108] requires immutable instance fields to be marked readonly so the compiler enforces immutability and intent is clear. HOW: Change the declaration to 'private readonly Action<CallAddedEvent> callAddedHandler = async delegate (CallAddedEvent message) { ... };'.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (message?.Call == null) | ||
| return; | ||
|
|
||
| var chatChannelService = ServiceLocator.Current.GetInstance<IChatChannelService>(); |
There was a problem hiding this comment.
WHAT: The same ServiceLocator.Current.GetInstance() lookup is repeated in every handler (lines 37, 53, 69, 100). WHY: Rule [14] requires duplicated sequences of statements to be extracted into a named function or utility; repeating the service-resolution boilerplate in four delegates is error-prone and harder to maintain. HOW: Inject IChatChannelService (and IIncidentCommandService) via the constructor as readonly fields, or extract a private helper such as 'IChatChannelService ResolveChat() => ServiceLocator.Current.GetInstance();' and call it from each handler.
Also found in:
Web/Resgrid.Web.Eventing/Worker.cs:207-208Core/Resgrid.Services/ChatChannelService.cs:215-221Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1393-1399Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:82-84Core/Resgrid.Services/ChatPermissionService.cs:90-90Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:299-310Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:307-309Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:193-198Core/Resgrid.Model/Chat/ChatModeration.cs:55-55Core/Resgrid.Services/ChatPermissionService.cs:101-101Core/Resgrid.Services/ChatPresenceService.cs:36-36Core/Resgrid.Services/ChatProvisioningEventService.cs:53-53Core/Resgrid.Services/ChatProvisioningEventService.cs:69-69Core/Resgrid.Services/ChatProvisioningEventService.cs:100-100Core/Resgrid.Services/ChatChannelService.cs:341-347Web/Resgrid.Web.Eventing/Worker.cs:220-221Web/Resgrid.Web.Eventing/Worker.cs:214-215Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:48-55Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:473-483Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:57-62Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:227-232Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:347-349Core/Resgrid.Model/Chat/ChatModeration.cs:105-105Core/Resgrid.Model/Chat/ChatModeration.cs:151-151Core/Resgrid.Model/Chat/ChatModeration.cs:203-203Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:261-266Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:380-382Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:430-432Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:488-490Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:519-521Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:555-557Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:590-592Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:994-996Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1051-1053Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1101-1103Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1172-1174Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1200-1202Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:370-377Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1092-1099Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:79-85Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:102-107Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:117-122Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:78-85Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:108-115Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:138-145Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:168-175Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:199-206Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:231-238Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:262-269Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:295-302Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:325-332Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:400-407Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:445-452Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:476-483Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:507-514Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:538-545Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:570-577Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:601-608Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:658-665Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:690-697Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:732-739Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:764-771Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:794-801Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:853-860Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:884-891Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:929-936Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:994-1001Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1047-1054Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1139-1146Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1186-1193Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1235-1242Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1280-1287Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1325-1332Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1356-1363Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1388-1395Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1436-1443Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1493-1500Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1538-1545Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1580-1587Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1610-1617
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Core/Resgrid.Services/ChatProvisioningEventService.cs:
Line 37:
WHAT: The same ServiceLocator.Current.GetInstance<IChatChannelService>() lookup is repeated in every handler (lines 37, 53, 69, 100). WHY: Rule [14] requires duplicated sequences of statements to be extracted into a named function or utility; repeating the service-resolution boilerplate in four delegates is error-prone and harder to maintain. HOW: Inject IChatChannelService (and IIncidentCommandService) via the constructor as readonly fields, or extract a private helper such as 'IChatChannelService ResolveChat() => ServiceLocator.Current.GetInstance<IChatChannelService>();' and call it from each handler.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| break; | ||
| case EventingTypes.ChatEvent: | ||
| if (ProcessChatEvent != null) | ||
| await ProcessChatEvent.Invoke(eventingMessage.DepartmentId, eventingMessage.Payload); |
There was a problem hiding this comment.
WHAT: The awaited invocation of ProcessChatEvent is not wrapped in a try/catch, so any exception thrown by the handler will propagate unhandled through the RabbitMQ message-processing pipeline. WHY: Rule [1] requires every awaited async operation to be guarded with error handling to prevent unhandled rejections from crashing or destabilizing the message consumer. HOW: Wrap the await in a try/catch block that logs the error with context (department id, event type) and either handles or rethrows the exception appropriately.
Also found in:
Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs:603-603Providers/Resgrid.Providers.Bus.Rabbit/RabbitTopicProvider.cs:123-123Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatPageElement.tsx:185-185Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:134-134Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:36-36Web/Resgrid.Web/Areas/User/Apps/src/components/chat/PinsPanel.tsx:48-48Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatApi.ts:50-50Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatApi.ts:257-257Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs:315-315Web/Resgrid.Web.Services/Controllers/v4/ChatbotController.cs:318-318Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatModerationApi.ts:150-150Providers/Resgrid.Providers.Messaging/NovuProvider.cs:391-391Workers/Resgrid.Workers.Console/Program.cs:462-462Workers/Resgrid.Workers.Console/Program.cs:456-456Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ThreadPanel.tsx:41-41Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatPanelElement.tsx:136-136Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ThreadPanel.tsx:72-72Core/Resgrid.Services/ChatNotificationService.cs:46-46Web/Resgrid.Web/Areas/User/Apps/src/components/chat/MembersPanel.tsx:22-22Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:60-60Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:117-117Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ExportsTab.tsx:97-97Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatModerationApi.ts:146-146Core/Resgrid.Services/PushService.cs:234-234Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:45-45Web/Resgrid.Web/Areas/User/Apps/src/components/chat/MembersPanel.tsx:36-36Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:33-33Providers/Resgrid.Providers.Messaging/NovuProvider.cs:396-396Providers/Resgrid.Providers.Messaging/NovuProvider.cs:401-401Core/Resgrid.Services/ChatPresenceService.cs:27-27Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ThreadPanel.tsx:73-73Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:62-62Core/Resgrid.Services/ChatNotificationService.cs:50-50Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatbotElement.tsx:33-33Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatbotElement.tsx:53-53Core/Resgrid.Services/ChatNotificationService.cs:93-93Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ThreadPanel.tsx:83-83Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:154-154Core/Resgrid.Services/ChatNotificationService.cs:56-56Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:197-197Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ThreadPanel.tsx:84-84Core/Resgrid.Services/ChatNotificationService.cs:54-54Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:84-89Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:213-213Core/Resgrid.Services/ChatNotificationService.cs:107-107Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:104-110Core/Resgrid.Services/PushService.cs:239-239Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:57-57Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:199-199Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:48-48Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts:244-244Core/Resgrid.Services/ChatPresenceService.cs:29-29Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:133-133Core/Resgrid.Services/ChatPresenceService.cs:41-41Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:119-126Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:200-200Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:61-61Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:201-201Core/Resgrid.Services/ChatPresenceService.cs:36-36Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:109-109Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:124-124Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:84-84Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:106-106Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:121-121Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:79-79Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:102-102Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:117-117
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File Providers/Resgrid.Providers.Bus.Rabbit/RabbitInboundEventProvider.cs:
Line 127:
WHAT: The awaited invocation of ProcessChatEvent is not wrapped in a try/catch, so any exception thrown by the handler will propagate unhandled through the RabbitMQ message-processing pipeline. WHY: Rule [1] requires every awaited async operation to be guarded with error handling to prevent unhandled rejections from crashing or destabilizing the message consumer. HOW: Wrap the await in a try/catch block that logs the error with context (department id, event type) and either handles or rethrows the exception appropriately.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .WithColumn("ParticipantType").AsInt32().NotNullable().WithDefaultValue(0) | ||
| .WithColumn("UserId").AsString(450).Nullable() | ||
| .WithColumn("UnitId").AsInt32().Nullable() | ||
| .WithColumn("DisplayNameOverride").AsString(int.MaxValue).Nullable() |
There was a problem hiding this comment.
WHAT: The DisplayNameOverride column is defined as NVARCHAR(MAX). WHY: Display names are short user-facing labels (typically <128 chars); NVARCHAR(MAX) is wasteful and harms performance. HOW: Use a bounded length such as AsString(128) or AsString(256).
Also found in:
Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:23-23Providers/Resgrid.Providers.Migrations/Migrations/M0105_AddChatMessages.cs:90-90Providers/Resgrid.Providers.Migrations/Migrations/M0105_AddChatMessages.cs:27-27Providers/Resgrid.Providers.Migrations/Migrations/M0105_AddChatMessages.cs:93-93Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:24-24Providers/Resgrid.Providers.Migrations/Migrations/M0105_AddChatMessages.cs:91-91
Kody rule violation: Optimize string column types
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:
Line 95:
WHAT: The DisplayNameOverride column is defined as NVARCHAR(MAX). WHY: Display names are short user-facing labels (typically <128 chars); NVARCHAR(MAX) is wasteful and harms performance. HOW: Use a bounded length such as AsString(128) or AsString(256).
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // FlagKey index. | ||
| Execute.Sql( | ||
| "IF NOT EXISTS (SELECT 1 FROM [FeatureFlags] WHERE [FlagKey] = '" + FlagKey + "') " + | ||
| "INSERT INTO [FeatureFlags] ([FlagKey], [Name], [Description], [Category], [IsEnabledGlobally]) " + |
There was a problem hiding this comment.
Violates team rule 'Prevent SQL Injection in Queries': Using unsanitized user input in SQL queries can lead to SQL injection attacks. Use parameterized queries to protect against malicious inputs.
Also found in:
Providers/Resgrid.Providers.MigrationsPg/Migrations/M0108_SeedChatFeatureFlagPg.cs:24-24Providers/Resgrid.Providers.MigrationsPg/Migrations/M0108_SeedChatFeatureFlagPg.cs:25-25
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0108_SeedChatFeatureFlag.cs:
Line 24:
Violates team rule 'Prevent SQL Injection in Queries': Using unsanitized user input in SQL queries can lead to SQL injection attacks. Use parameterized queries to protect against malicious inputs.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // Keep FlagKey in sync with Resgrid.Model.FeatureFlagKeys.ChatSystem. | ||
| private const string FlagKey = "Chat.System"; |
There was a problem hiding this comment.
WHAT: The "Chat.System" flag key is redefined as a local constant here even though Resgrid.Model.FeatureFlagKeys.ChatSystem is the canonical source — the comment itself admits the two must be manually kept in sync. WHY: Rule [6] requires shared string literals (keys, claim types, flag keys) to live in a single place; duplicating them invites drift where the migration seeds one value while runtime code checks another, silently breaking the feature gate. HOW: If migration immutability prevents referencing the model constant directly, keep the hardcoded value but add a unit/integration test (or a build-time check) asserting that the literal seeded here equals FeatureFlagKeys.ChatSystem, so drift is caught automatically instead of by a comment.
Also found in:
Providers/Resgrid.Providers.MigrationsPg/Migrations/M0108_SeedChatFeatureFlagPg.cs:13-13Web/Resgrid.Web.Services/Startup.cs:760-760Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs:760-760Web/Resgrid.Web.Eventing/Worker.cs:207-207Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1179-1179Web/Resgrid.Web.Eventing/Startup.cs:303-303Core/Resgrid.Services/ChatPermissionService.cs:648-648Core/Resgrid.Services/ChatPermissionService.cs:643-643Web/Resgrid.Web.Eventing/Startup.cs:393-393Core/Resgrid.Model/Chat/ChatModeration.cs:55-55Web/Resgrid.Web.Services/Hubs/ChatHub.cs:42-42Web/Resgrid.Web.Services/Hubs/ChatHub.cs:47-47Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.ts:375-375Core/Resgrid.Services/ChatPresenceService.cs:62-62Providers/Resgrid.Providers.Messaging/GifProvider.cs:29-29Providers/Resgrid.Providers.Messaging/GifProvider.cs:32-32Web/Resgrid.Web.Eventing/Worker.cs:214-214Web/Resgrid.Web.Eventing/Worker.cs:220-220Core/Resgrid.Model/Chat/ChatModeration.cs:105-105Core/Resgrid.Model/Chat/ChatModeration.cs:151-151Core/Resgrid.Model/Chat/ChatModeration.cs:203-203Core/Resgrid.Services/ChatMessageService.cs:399-399Core/Resgrid.Services/ChatMessageService.cs:369-369Providers/Resgrid.Providers.Messaging/GifProvider.cs:46-46Providers/Resgrid.Providers.Messaging/GifProvider.cs:65-65Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:42-42Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1207-1207Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:64-64Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:43-43Web/Resgrid.Web.Services/Hubs/ChatHub.cs:87-87Web/Resgrid.Web.Services/Hubs/ChatHub.cs:49-49Web/Resgrid.Web.Services/Hubs/ChatHub.cs:71-71Web/Resgrid.Web.Services/Hubs/ChatHub.cs:43-43Web/Resgrid.Web.Services/Hubs/ChatHub.cs:66-66Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:47-47Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:71-71Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:87-87Web/Resgrid.Web.Services/Hubs/ChatHub.cs:64-64
Kody rule violation: Centralize string constants
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0108_SeedChatFeatureFlag.cs:
Line 12 to 13:
WHAT: The "Chat.System" flag key is redefined as a local constant here even though Resgrid.Model.FeatureFlagKeys.ChatSystem is the canonical source — the comment itself admits the two must be manually kept in sync. WHY: Rule [6] requires shared string literals (keys, claim types, flag keys) to live in a single place; duplicating them invites drift where the migration seeds one value while runtime code checks another, silently breaking the feature gate. HOW: If migration immutability prevents referencing the model constant directly, keep the hardcoded value but add a unit/integration test (or a build-time check) asserting that the literal seeded here equals FeatureFlagKeys.ChatSystem, so drift is caught automatically instead of by a comment.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Execute.Sql("CREATE UNIQUE INDEX IF NOT EXISTS ux_chatchannels_node ON chatchannels (commandstructurenodeid) WHERE commandstructurenodeid IS NOT NULL;"); | ||
|
|
||
| // At most one chatbot conversation per user per department (ChannelType 8 = Chatbot). | ||
| Execute.Sql("CREATE UNIQUE INDEX IF NOT EXISTS ux_chatchannels_department_owner_bot ON chatchannels (departmentid, owneruserid) WHERE channeltype = 8 AND owneruserid IS NOT NULL;"); |
There was a problem hiding this comment.
WHAT: The magic number 8 is used in raw SQL to filter for Chatbot channel type. WHY: Numeric literals with domain meaning are opaque and error-prone; if the enum value changes or someone misreads the number, bugs are introduced silently. HOW: Define a named constant (e.g., const int ChannelTypeChatbot = 8;) in the migration or a shared constants class and interpolate it into the SQL string.
Also found in:
Core/Resgrid.Services/ChatNotificationService.cs:151-151Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ActionsTab.tsx:66-66Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatFormat.ts:152-152Core/Resgrid.Services/ChatModerationService.cs:78-78Providers/Resgrid.Providers.MigrationsPg/Migrations/M0104_AddChatChannelsPg.cs:65-65Providers/Resgrid.Providers.MigrationsPg/Migrations/M0104_AddChatChannelsPg.cs:62-62Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChannelList.tsx:52-52Workers/Resgrid.Workers.Console/Program.cs:457-457Providers/Resgrid.Providers.MigrationsPg/Migrations/M0104_AddChatChannelsPg.cs:123-123Workers/Resgrid.Workers.Console/Program.cs:463-463Core/Resgrid.Services/ChatModerationService.cs:192-192Providers/Resgrid.Providers.MigrationsPg/Migrations/M0104_AddChatChannelsPg.cs:120-120Workers/Resgrid.Workers.Console/Program.cs:458-458Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.ts:226-226Providers/Resgrid.Providers.Messaging/GifProvider.cs:22-22Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/MessageBubble.tsx:52-52Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:59-59Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:550-550Workers/Resgrid.Workers.Console/Program.cs:464-464Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatActions.ts:40-40Providers/Resgrid.Providers.Messaging/GifProvider.cs:127-127Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatApi.ts:273-273Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:65-65Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.ts:379-379Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:101-101Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.ts:358-358Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:174-174Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatApi.ts:162-162Providers/Resgrid.Providers.Messaging/GifProvider.cs:125-125Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:62-62Core/Resgrid.Chatbot/Interfaces/IChatCompletionClient.cs:33-33Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsx:114-114Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:120-120Providers/Resgrid.Providers.Migrations/Migrations/M0104_AddChatChannels.cs:123-123Core/Resgrid.Services/ChatMessageService.cs:437-437Core/Resgrid.Model/Chat/ChatModeration.cs:44-44Core/Resgrid.Services/ChatPresenceService.cs:67-67Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:88-88Core/Resgrid.Chatbot.NLU/Providers/OpenAiCompatibleChatCompletionClient.cs:108-108Core/Resgrid.Model/Chat/ChatChannel.cs:80-80Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:87-87Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1228-1228Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatModerationApi.ts:99-99Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ExportsTab.tsx:91-91Core/Resgrid.Services/ChatMessageService.cs:272-272Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatModerationApi.ts:73-73Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/ExportsTab.tsx:96-96Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsx:116-116Web/Resgrid.Web/Areas/User/Apps/src/components/chat/moderation/FlagsTab.tsx:115-115Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:1257-1257Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:70-70Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:107-107Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ChatPanelElement.tsx:54-54Core/Resgrid.Model/Chat/ChatMessage.cs:83-83Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:132-132Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:133-133Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:100-100Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:120-120Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatStore.ts:276-276Core/Resgrid.Model/Chat/ChatChannel.cs:126-126Core/Resgrid.Model/Chat/ChatChannel.cs:202-202Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:152-152Core/Resgrid.Services/ChatMessageService.cs:591-591Core/Resgrid.Model/Chat/ChatMessage.cs:124-124Core/Resgrid.Model/Chat/ChatMessage.cs:179-179Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatModerationApi.ts:158-158Core/Resgrid.Services/ChatMessageService.cs:588-588Web/Resgrid.Web/Areas/User/Apps/src/components/chat/ConversationView.tsx:101-101Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:585-585Web/Resgrid.Web.Services/Controllers/v4/ChatController.cs:619-619Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:162-162Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:163-163Core/Resgrid.Model/Chat/ChatModeration.cs:94-94Core/Resgrid.Model/Chat/ChatModeration.cs:140-140Core/Resgrid.Model/Chat/ChatModeration.cs:192-192Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1213-1213Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1220-1220Web/Resgrid.Web/Areas/User/Apps/src/components/chat/atoms/Composer.tsx:122-122Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1574-1574Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:1575-1575Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:193-193Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:194-194Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:532-532Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:533-533
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0104_AddChatChannelsPg.cs:
Line 59:
WHAT: The magic number 8 is used in raw SQL to filter for Chatbot channel type. WHY: Numeric literals with domain meaning are opaque and error-prone; if the enum value changes or someone misreads the number, bugs are introduced silently. HOW: Define a named constant (e.g., `const int ChannelTypeChatbot = 8;`) in the migration or a shared constants class and interpolate it into the SQL string.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| parameters.Add("DepartmentId", departmentId); | ||
| parameters.Add("Status", status); | ||
| parameters.Add("PageSize", pageSize); | ||
| parameters.Add("Offset", (page - 1) * pageSize); |
There was a problem hiding this comment.
Negative OFFSET causes a SQL error on the default first-page request. The controllers default page = 0 (ChatModerationController.cs:65,281; ChatController.cs:1228) and the services clamp with Math.Max(page, 0) which keeps 0, but the repositories compute Offset = (page - 1) * pageSize, yielding -pageSize. T-SQL 'OFFSET ... ROWS' and Postgres 'OFFSET' both reject negative values, so every default GetFlags/GetActions/Search call throws instead of returning the first page. Use Math.Max(0, page - 1) * pageSize (or have the callers pass a 1-based page) so the first page resolves to offset 0.
parameters.Add("Offset", Math.Max(0, page - 1) * pageSize);Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:
Line 1427:
Negative OFFSET causes a SQL error on the default first-page request. The controllers default page = 0 (ChatModerationController.cs:65,281; ChatController.cs:1228) and the services clamp with Math.Max(page, 0) which keeps 0, but the repositories compute Offset = (page - 1) * pageSize, yielding -pageSize. T-SQL 'OFFSET ... ROWS' and Postgres 'OFFSET' both reject negative values, so every default GetFlags/GetActions/Search call throws instead of returning the first page. Use Math.Max(0, page - 1) * pageSize (or have the callers pass a 1-based page) so the first page resolves to offset 0.
Suggested Code:
parameters.Add("Offset", Math.Max(0, page - 1) * pageSize);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| parameters.Add("Ids", ids); | ||
| parameters.Add("Query", $"%{escaped}%"); | ||
| parameters.Add("PageSize", pageSize); | ||
| parameters.Add("Offset", (page - 1) * pageSize); |
There was a problem hiding this comment.
WHAT: Negative OFFSET in paginated queries causes a SQL error on the default first-page request. WHY: The controllers (ChatController.Search page=0, ChatModerationController.GetFlags page=0, ChatModerationController.GetActions page=0) all default page to 0, but the repository computes Offset = (page - 1) * pageSize, producing -50; both PostgreSQL (OFFSET -50) and SQL Server (OFFSET -50 ROWS FETCH NEXT) reject negative offsets with a runtime SQL error, breaking every default first-page call. HOW: Clamp the page value in the controllers to a minimum of 1 (e.g., page = Math.Max(1, page)) before passing to the service, or clamp in the repository (Offset = Math.Max(0, (page - 1) * pageSize)).
parameters.Add("Offset", Math.Max(0, (page - 1) * pageSize));Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:
Line 829:
WHAT: Negative OFFSET in paginated queries causes a SQL error on the default first-page request. WHY: The controllers (ChatController.Search page=0, ChatModerationController.GetFlags page=0, ChatModerationController.GetActions page=0) all default page to 0, but the repository computes Offset = (page - 1) * pageSize, producing -50; both PostgreSQL (OFFSET -50) and SQL Server (OFFSET -50 ROWS FETCH NEXT) reject negative offsets with a runtime SQL error, breaking every default first-page call. HOW: Clamp the page value in the controllers to a minimum of 1 (e.g., page = Math.Max(1, page)) before passing to the service, or clamp in the repository (Offset = Math.Max(0, (page - 1) * pageSize)).
Suggested Code:
parameters.Add("Offset", Math.Max(0, (page - 1) * pageSize));
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // The ChatMessages delete is last; its affected count is the number of messages purged. | ||
| var lastAffected = 0; | ||
| foreach (var statement in statements) | ||
| lastAffected = await connection.ExecuteAsync(statement, parameters, _unitOfWork.Transaction); |
There was a problem hiding this comment.
WHAT: DeleteMessagesByIdsAsync executes 7 DELETE statements in a loop; when _unitOfWork.Connection is null a standalone connection is used with a null transaction, so partial failures leave orphaned child rows. WHY: Multi-step writes must be atomic to avoid inconsistent data. HOW: Wrap the loop in an explicit transaction (BeginTransaction/Commit/Rollback) when _unitOfWork.Transaction is null, ensuring all deletes succeed or all roll back.
Also found in:
Core/Resgrid.Services/ChatModerationService.cs:242-242Core/Resgrid.Services/ChatModerationService.cs:93-93Core/Resgrid.Services/ChatModerationService.cs:109-109Core/Resgrid.Services/ChatModerationService.cs:131-131Core/Resgrid.Services/ChatChannelService.cs:335-356Core/Resgrid.Services/ChatModerationService.cs:179-179Core/Resgrid.Services/ChatModerationService.cs:158-158Core/Resgrid.Services/ChatMessageService.cs:146-146Core/Resgrid.Services/ChatModerationService.cs:198-198Core/Resgrid.Services/ChatChannelService.cs:179-192Core/Resgrid.Services/ChatChannelService.cs:207-228Core/Resgrid.Services/ChatMessageService.cs:255-255Core/Resgrid.Services/ChatChannelService.cs:281-308Core/Resgrid.Services/ChatMessageService.cs:230-230Core/Resgrid.Services/ChatMessageService.cs:155-155Core/Resgrid.Services/ChatMessageService.cs:158-158
Kody rule violation: Handle transaction rollbacks properly
Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:
Line 989:
WHAT: DeleteMessagesByIdsAsync executes 7 DELETE statements in a loop; when _unitOfWork.Connection is null a standalone connection is used with a null transaction, so partial failures leave orphaned child rows. WHY: Multi-step writes must be atomic to avoid inconsistent data. HOW: Wrap the loop in an explicit transaction (BeginTransaction/Commit/Rollback) when _unitOfWork.Transaction is null, ensuring all deletes succeed or all roll back.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public class when_ensuring_chatbot_channels : with_the_chat_channel_service | ||
| { | ||
| [Test] | ||
| public async Task missing_chatbot_channel_should_create_channel_with_owner_and_bot_members() |
There was a problem hiding this comment.
WHAT: The method name uses snake_case instead of PascalCase. WHY: .NET naming conventions require PascalCase for methods. HOW: Rename to MissingChatbotChannelShouldCreateChannelWithOwnerAndBotMembers.
Also found in:
Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:272-272Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:104-104Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:173-173Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:86-86Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:203-203Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:304-304Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:129-129Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:253-253Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:235-235Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:336-336Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:200-200Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:222-222Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:301-301Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:289-289Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:324-324Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:83-83Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:321-321Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:269-269Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:150-150Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:18-18Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:126-126
Kody rule violation: Use proper naming conventions
Prompt for LLM
File Tests/Resgrid.Tests/Services/ChatChannelServiceTests.cs:
Line 153:
WHAT: The method name uses snake_case instead of PascalCase. WHY: .NET naming conventions require PascalCase for methods. HOW: Rename to `MissingChatbotChannelShouldCreateChannelWithOwnerAndBotMembers`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var departmentId = ClaimsAuthorizationHelper.GetDepartmentId(); | ||
| var userId = ClaimsAuthorizationHelper.GetUserId(); | ||
|
|
||
| var channel = await _chatChannelService.GetChannelByIdAsync(channelId); |
There was a problem hiding this comment.
WHAT: Database query issued before validating that departmentId and userId are valid. WHY: Connect validates departmentId > 0 on line 39, but JoinChannel queries the DB first and only checks department ownership afterward, wasting a query for invalid calls. HOW: Add 'if (departmentId <= 0 || string.IsNullOrWhiteSpace(userId)) return;' before the channel fetch.
Also found in:
Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:79-79Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:102-102Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:117-117
Kody rule violation: Order validations before database queries
Prompt for LLM
File Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:
Line 57:
WHAT: Database query issued before validating that departmentId and userId are valid. WHY: Connect validates departmentId > 0 on line 39, but JoinChannel queries the DB first and only checks department ownership afterward, wasting a query for invalid calls. HOW: Add 'if (departmentId <= 0 || string.IsNullOrWhiteSpace(userId)) return;' before the channel fetch.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var departmentId = Context.Items.TryGetValue(DepartmentIdContextKey, out var departmentIdValue) && departmentIdValue is int id ? id : 0; | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(userId) && RemoveUserConnection(userId, Context.ConnectionId) && departmentId > 0) | ||
| await Clients.Group($"chatdept:{departmentId}").SendAsync("chatPresenceChanged", userId, false); |
There was a problem hiding this comment.
Unhandled exception risk identified in the awaited Clients.Group(...).SendAsync SignalR call. Network operations can throw transport errors, and an unhandled exception in OnDisconnectedAsync cascades and leaves stale presence state. Wrap the call in a try/catch block with structured logging for operation context.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:
Line 75:
Unhandled exception risk identified in the awaited `Clients.Group(...).SendAsync` SignalR call. Network operations can throw transport errors, and an unhandled exception in `OnDisconnectedAsync` cascades and leaves stale presence state. Wrap the call in a `try/catch` block with structured logging for operation context.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var valid = await _userManager.VerifyTwoFactorTokenAsync( | ||
| user, | ||
| _userManager.Options.Tokens.AuthenticatorTokenProvider, | ||
| input.TotpCode.Trim()); | ||
|
|
||
| var result = new ChatActionResult(); | ||
|
|
||
| if (!valid) | ||
| { | ||
| result.Success = false; | ||
| result.Status = ResponseHelper.Failure; | ||
| ResponseHelper.PopulateV4ResponseData(result); | ||
| return StatusCode(StatusCodes.Status401Unauthorized, result); | ||
| } |
There was a problem hiding this comment.
Brute-force vulnerability identified in the VerifyExportMfa endpoint. The _userManager.VerifyTwoFactorTokenAsync call lacks rate limiting and lockout enforcement, allowing attackers to brute-force the 6-digit TOTP code and bypass MFA protection. Add a per-user rate limit via the existing _cacheProvider.IncrementAsync pattern and invoke _userManager.AccessFailedAsync(user) on invalid attempts.
// Rate-limit TOTP attempts per user to prevent brute-force of the 6-digit code
var attemptKey = $"chat:rl:mfa:{user.Id}";
var attempts = await _cacheProvider.IncrementAsync(attemptKey, TimeSpan.FromMinutes(1));
if (attempts > 5)
{
await _userManager.AccessFailedAsync(user);
return StatusCode(StatusCodes.Status429TooManyRequests, new { error = "rate_limited" });
}
var valid = await _userManager.VerifyTwoFactorTokenAsync(
user,
_userManager.Options.Tokens.AuthenticatorTokenProvider,
input.TotpCode.Trim());
var result = new ChatActionResult();
if (!valid)
{
await _userManager.AccessFailedAsync(user);
result.Success = false;
result.Status = ResponseHelper.Failure;
ResponseHelper.PopulateV4ResponseData(result);
return StatusCode(StatusCodes.Status401Unauthorized, result);
}Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:
Line 613 to 626:
Brute-force vulnerability identified in the `VerifyExportMfa` endpoint. The `_userManager.VerifyTwoFactorTokenAsync` call lacks rate limiting and lockout enforcement, allowing attackers to brute-force the 6-digit TOTP code and bypass MFA protection. Add a per-user rate limit via the existing `_cacheProvider.IncrementAsync` pattern and invoke `_userManager.AccessFailedAsync(user)` on invalid attempts.
Suggested Code:
// Rate-limit TOTP attempts per user to prevent brute-force of the 6-digit code
var attemptKey = $"chat:rl:mfa:{user.Id}";
var attempts = await _cacheProvider.IncrementAsync(attemptKey, TimeSpan.FromMinutes(1));
if (attempts > 5)
{
await _userManager.AccessFailedAsync(user);
return StatusCode(StatusCodes.Status429TooManyRequests, new { error = "rate_limited" });
}
var valid = await _userManager.VerifyTwoFactorTokenAsync(
user,
_userManager.Options.Tokens.AuthenticatorTokenProvider,
input.TotpCode.Trim());
var result = new ChatActionResult();
if (!valid)
{
await _userManager.AccessFailedAsync(user);
result.Success = false;
result.Status = ResponseHelper.Failure;
ResponseHelper.PopulateV4ResponseData(result);
return StatusCode(StatusCodes.Status401Unauthorized, result);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| /// <summary> | ||
| /// Establishes a recent-MFA step-up proof for chat transcript exports. Verifies the caller's current | ||
| /// authenticator (TOTP) code and, on success, records a server-side proof valid for a short window so |
There was a problem hiding this comment.
Unsafe type casting identified in the codebase. Direct casts bypass compile-time type safety. Replace direct casts with the as operator or pattern matching, and guard null results before usage.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:
Line 583:
Unsafe type casting identified in the codebase. Direct casts bypass compile-time type safety. Replace direct casts with the `as` operator or pattern matching, and guard `null` results before usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| // A caller with no 2FA enrolled can never obtain a proof — exports stay blocked until they enroll. | ||
| if (!await _userManager.GetTwoFactorEnabledAsync(user)) | ||
| return StatusCode(StatusCodes.Status403Forbidden, new { error = "mfa_enrollment_required", error_description = "Two-Factor Authentication must be enabled to export chat transcripts." }); |
There was a problem hiding this comment.
Maintenance hazard introduced by the inline magic strings "mfa_enrollment_required" and its description. Duplicating shared string literals invites typos and inconsistency across the codebase. Define constants such as private const string MfaEnrollmentRequiredError = "mfa_enrollment_required"; to centralize these values.
Kody rule violation: Centralize string constants
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:
Line 611:
Maintenance hazard introduced by the inline magic strings `"mfa_enrollment_required"` and its description. Duplicating shared string literals invites typos and inconsistency across the codebase. Define constants such as `private const string MfaEnrollmentRequiredError = "mfa_enrollment_required";` to centralize these values.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| // A caller with no 2FA enrolled can never obtain a proof — exports stay blocked until they enroll. | ||
| if (!await _userManager.GetTwoFactorEnabledAsync(user)) | ||
| return StatusCode(StatusCodes.Status403Forbidden, new { error = "mfa_enrollment_required", error_description = "Two-Factor Authentication must be enabled to export chat transcripts." }); |
There was a problem hiding this comment.
Type safety risk identified in the string literal "mfa_enrollment_required". Using raw strings to represent a finite set of export error types invites typos and bypasses compile-time checks. Define an enum, such as ExportErrorCode { MfaEnrollmentRequired, MfaRequired }, to represent these states.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:
Line 611:
Type safety risk identified in the string literal `"mfa_enrollment_required"`. Using raw strings to represent a finite set of export error types invites typos and bypasses compile-time checks. Define an enum, such as `ExportErrorCode { MfaEnrollmentRequired, MfaRequired }`, to represent these states.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var user = await _userManager.GetUserAsync(User); | ||
| if (user == null) | ||
| return Unauthorized(); | ||
|
|
||
| if (!await _userManager.GetTwoFactorEnabledAsync(user)) | ||
| return StatusCode(StatusCodes.Status403Forbidden, new { error = "mfa_enrollment_required", error_description = "Two-Factor Authentication must be enabled to export chat transcripts." }); |
There was a problem hiding this comment.
Code duplication identified in the 2FA enrollment verification sequence. The identical block of retrieving the user, null-checking, and returning a 403 violates DRY principles and increases maintenance burden. Extract a shared helper method such as GetMfaEnrolledUserOrErrorAsync() that returns either the user or an error ActionResult.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:
Line 646 to 651:
Code duplication identified in the 2FA enrollment verification sequence. The identical block of retrieving the user, null-checking, and returning a 403 violates DRY principles and increases maintenance burden. Extract a shared helper method such as `GetMfaEnrolledUserOrErrorAsync()` that returns either the user or an error `ActionResult`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var user = await _userManager.GetUserAsync(User); | ||
| if (user == null) | ||
| return Unauthorized(); | ||
|
|
||
| if (!await _userManager.GetTwoFactorEnabledAsync(user)) | ||
| return StatusCode(StatusCodes.Status403Forbidden, new { error = "mfa_enrollment_required", error_description = "Two-Factor Authentication must be enabled to export chat transcripts." }); |
There was a problem hiding this comment.
Business logic duplication identified in the inline MFA-enrollment enforcement. Implementing domain rules directly in controller methods prevents consistent enforcement and violates separation of concerns. Move the enrollment check into a shared private helper or dedicated MFA service method that both VerifyExportMfa and CheckRecentExportMfaAsync invoke.
Kody rule violation: Extract duplicated business logic
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:
Line 646 to 651:
Business logic duplication identified in the inline MFA-enrollment enforcement. Implementing domain rules directly in controller methods prevents consistent enforcement and violates separation of concerns. Move the enrollment check into a shared private helper or dedicated MFA service method that both `VerifyExportMfa` and `CheckRecentExportMfaAsync` invoke.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// The caller's current authenticator (TOTP) code, used to establish a recent-MFA step-up proof for PII exports | ||
| /// </summary> | ||
| [Required] | ||
| public string TotpCode { get; set; } |
There was a problem hiding this comment.
Uninitialized state risk identified in the TotpCode auto-property. Omitting default values for class properties violates Rule [31] and introduces null reference risks. Initialize the property to provide deterministic state, such as public string TotpCode { get; set; } = string.Empty;.
Kody rule violation: Initialize properties with default values
Prompt for LLM
File Web/Resgrid.Web.Services/Models/v4/Chat/ChatApiModels.cs:
Line 1427:
Uninitialized state risk identified in the `TotpCode` auto-property. Omitting default values for class properties violates Rule [31] and introduces null reference risks. Initialize the property to provide deterministic state, such as `public string TotpCode { get; set; } = string.Empty;`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, $"{Name}: Error processing Personnel Location queue item with an id of {personnelLocationEvent?.EventId}."); |
There was a problem hiding this comment.
Structured logging violation identified in the _logger.LogError call. String interpolation generates a flat message string and prevents identifiers from being captured as queryable fields in the logging backend. Replace the interpolated string with a message template using named placeholders, such as "Error processing Personnel Location queue item with EventId {EventId} for {WorkerName}".
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Workers/Resgrid.Workers.Console/Tasks/PersonnelLocationQueuesProcessorTask.cs:
Line 65:
Structured logging violation identified in the `_logger.LogError` call. String interpolation generates a flat message string and prevents identifiers from being captured as queryable fields in the logging backend. Replace the interpolated string with a message template using named placeholders, such as `"Error processing Personnel Location queue item with EventId {EventId} for {WorkerName}"`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| yield return "# ExportId: " + ExportId; | ||
| yield return "# DepartmentId: " + DepartmentId.ToString(CultureInfo.InvariantCulture); | ||
| yield return "# RequestedByUserId: " + RequestedByUserId; | ||
| yield return "# RequestedOnUtc: " + RequestedOn.ToString("O", CultureInfo.InvariantCulture); |
There was a problem hiding this comment.
Readability issue caused by utilizing + for string concatenation over interpolation. Manual concatenation is error-prone and decreases code maintainability. Replace the string concatenation with an interpolated string, such as $"# RequestedOnUtc: {RequestedOn.ToString(\"O\", CultureInfo.InvariantCulture)}".
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File Workers/Resgrid.Workers.Framework/Logic/ChatExportLogic.cs:
Line 289:
Readability issue caused by utilizing `+` for string concatenation over interpolation. Manual concatenation is error-prone and decreases code maintainability. Replace the string concatenation with an interpolated string, such as `$"# RequestedOnUtc: {RequestedOn.ToString(\"O\", CultureInfo.InvariantCulture)}"`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This comment has been minimized.
This comment has been minimized.
|
|
||
| // Batch-validate all memberships in a single query instead of one round trip per member. | ||
| var membersInDepartment = await _departmentsService.GetMemberUserIdsInDepartmentAsync(departmentId, validatedMemberIds); | ||
| if (validatedMemberIds.Any(id => !membersInDepartment.Contains(id))) |
There was a problem hiding this comment.
O(n*m) complexity in ChatChannelService: membersInDepartment.Contains(id) executes inside an .Any() loop on a List. Convert membersInDepartment to a HashSet before the Any check so each lookup is O(1).
Kody rule violation: Use HashSet for fast membership checks
Prompt for LLM
File Core/Resgrid.Services/ChatChannelService.cs:
Line 230:
O(n*m) complexity in ChatChannelService: membersInDepartment.Contains(id) executes inside an .Any() loop on a List. Convert membersInDepartment to a HashSet<string> before the Any check so each lookup is O(1).
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var members = await _departmentMembersRepository.GetAllDepartmentMembersUnlimitedAsync(departmentId); | ||
| if (members != null) | ||
| { | ||
| foreach (var member in members) | ||
| { | ||
| if (member?.UserId != null && candidates.Contains(member.UserId)) | ||
| result.Add(member.UserId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Performance regression in GetMemberUserIdsInDepartmentAsync: replacing the cached per-member path (IsUserInDepartmentAsync → GetDepartmentMemberAsync) with GetAllDepartmentMembersUnlimitedAsync loads thousands of DepartmentMember+IdentityUser records via Dapper QueryAsync<DepartmentMember, IdentityUser, DepartmentMember> on every channel-creation call. Replace the full-list load with a targeted query filtering by DepartmentId and candidate UserIds (e.g., SELECT UserId FROM DepartmentMembers WHERE DepartmentId=@deptID AND UserId IN @candidates).
// Use a targeted query that filters at the DB level by candidate UserIds
// rather than loading the entire department member list with User objects.
// e.g. await _departmentMembersRepository.GetMembersByUserIdsAsync(departmentId, candidates);Prompt for LLM
File Core/Resgrid.Services/DepartmentsService.cs:
Line 782 to 790:
Performance regression in GetMemberUserIdsInDepartmentAsync: replacing the cached per-member path (IsUserInDepartmentAsync → GetDepartmentMemberAsync) with GetAllDepartmentMembersUnlimitedAsync loads thousands of DepartmentMember+IdentityUser records via Dapper QueryAsync<DepartmentMember, IdentityUser, DepartmentMember> on every channel-creation call. Replace the full-list load with a targeted query filtering by DepartmentId and candidate UserIds (e.g., SELECT UserId FROM DepartmentMembers WHERE DepartmentId=@deptId AND UserId IN @candidates).
Suggested Code:
// Use a targeted query that filters at the DB level by candidate UserIds
// rather than loading the entire department member list with User objects.
// e.g. await _departmentMembersRepository.GetMembersByUserIdsAsync(departmentId, candidates);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var candidates = userIds? | ||
| .Where(id => !string.IsNullOrWhiteSpace(id)) | ||
| .ToHashSet(StringComparer.Ordinal); | ||
|
|
||
| var result = new HashSet<string>(StringComparer.Ordinal); |
There was a problem hiding this comment.
False rejection in GetMemberUserIdsInDepartmentAsync: candidates.Contains(member.UserId) uses StringComparer.Ordinal while the prior SQL path relied on SQL Server's default CI_AS collation. User IDs arriving with different casing fail to match, causing ChatChannelService.CreateAdHocGroupChannelAsync to throw UnauthorizedAccessException for valid department members. Use StringComparer.OrdinalIgnoreCase for both the candidates and result HashSets to preserve case-insensitive matching.
var candidates = userIds?
.Where(id => !string.IsNullOrWhiteSpace(id))
.ToHashSet(StringComparer.OrdinalIgnoreCase);
var result = new HashSet<string>(StringComparer.OrdinalIgnoreCase);Prompt for LLM
File Core/Resgrid.Services/DepartmentsService.cs:
Line 773 to 777:
False rejection in GetMemberUserIdsInDepartmentAsync: candidates.Contains(member.UserId) uses StringComparer.Ordinal while the prior SQL path relied on SQL Server's default CI_AS collation. User IDs arriving with different casing fail to match, causing ChatChannelService.CreateAdHocGroupChannelAsync to throw UnauthorizedAccessException for valid department members. Use StringComparer.OrdinalIgnoreCase for both the candidates and result HashSets to preserve case-insensitive matching.
Suggested Code:
var candidates = userIds?
.Where(id => !string.IsNullOrWhiteSpace(id))
.ToHashSet(StringComparer.OrdinalIgnoreCase);
var result = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return result; | ||
|
|
||
| // One query for the department's members (no per-user round trips), filtered to the candidates. | ||
| var members = await _departmentMembersRepository.GetAllDepartmentMembersUnlimitedAsync(departmentId); |
There was a problem hiding this comment.
Unguarded async operation in DepartmentsService.cs: the awaited call to GetAllDepartmentMembersUnlimitedAsync lacks try/catch, risking unhandled rejection. Wrap the await in a try/catch block and log the error with departmentId and operation context.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File Core/Resgrid.Services/DepartmentsService.cs:
Line 782:
Unguarded async operation in DepartmentsService.cs: the awaited call to GetAllDepartmentMembersUnlimitedAsync lacks try/catch, risking unhandled rejection. Wrap the await in a try/catch block and log the error with departmentId and operation context.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // Belt-and-suspenders scrub for key=/api_key= query params. A bounded match timeout caps regex work | ||
| // on pathological input (ReDoS guard); on timeout the literal-key replacements have already removed | ||
| // the real secrets, so falling through without the query-param scrub is safe. | ||
| private static readonly Regex KeyQueryParamRegex = new Regex( |
There was a problem hiding this comment.
Denial-of-Service vulnerability in GifProvider.cs: KeyQueryParamRegex is instantiated without a match timeout, allowing untrusted input to trigger runaway regex evaluation. Define a timeout when constructing the Regex. Also found at GifProvider.cs:122.
Kody rule violation: Specify Timeout for Regular Expressions
Prompt for LLM
File Providers/Resgrid.Providers.Messaging/GifProvider.cs:
Line 103:
Denial-of-Service vulnerability in GifProvider.cs: KeyQueryParamRegex is instantiated without a match timeout, allowing untrusted input to trigger runaway regex evaluation. Define a timeout when constructing the Regex. Also found at GifProvider.cs:122.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| catch (RegexMatchTimeoutException) | ||
| { | ||
| // Query-param scrub timed out; literal key replacements above already removed the secrets. | ||
| } |
There was a problem hiding this comment.
Swallowed exception in GifProvider.cs: the catch (RegexMatchTimeoutException) block contains only a comment with no logging or rethrow, hiding operational signals such as ReDoS attempts. Capture the exception variable and log it with context, e.g. Logging.LogError($"KeyQueryParamRegex timed out after {KeyQueryParamRegex.MatchTimeout}; falling back to literal-key sanitization. Detail: {ex.Message}").
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File Providers/Resgrid.Providers.Messaging/GifProvider.cs:
Line 124 to 127:
Swallowed exception in GifProvider.cs: the catch (RegexMatchTimeoutException) block contains only a comment with no logging or rethrow, hiding operational signals such as ReDoS attempts. Capture the exception variable and log it with context, e.g. Logging.LogError($"KeyQueryParamRegex timed out after {KeyQueryParamRegex.MatchTimeout}; falling back to literal-key sanitization. Detail: {ex.Message}").
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| private static readonly Regex KeyQueryParamRegex = new Regex( | ||
| @"\b(api_key|key)=[^&\s""']+", | ||
| RegexOptions.IgnoreCase | RegexOptions.CultureInvariant, | ||
| TimeSpan.FromMilliseconds(250)); |
There was a problem hiding this comment.
Magic number in GifProvider.cs: the regex match timeout value 250 is an inline literal. Extract it to a named constant such as private static readonly TimeSpan KeyScrubMatchTimeout = TimeSpan.FromMilliseconds(250) for self-documenting, single-source tuning.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File Providers/Resgrid.Providers.Messaging/GifProvider.cs:
Line 106:
Magic number in GifProvider.cs: the regex match timeout value 250 is an inline literal. Extract it to a named constant such as private static readonly TimeSpan KeyScrubMatchTimeout = TimeSpan.FromMilliseconds(250) for self-documenting, single-source tuning.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| // Best-effort presence broadcast: the connection is already removed, so a transport | ||
| // failure here must not abort the disconnect flow. Log with context and continue. | ||
| Resgrid.Framework.Logging.LogException(ex, $"ChatHub presence-offline broadcast failed for user {userId} in department {departmentId}."); |
There was a problem hiding this comment.
Unstructured log fields in ChatHub: LogException receives userId and departmentId via string interpolation instead of as separate structured parameters, preventing searchable filtering in SIEM tools. Pass identifiers as structured fields, e.g. Resgrid.Framework.Logging.LogException(ex, "ChatHub presence-offline broadcast failed for user {UserId} in department {DepartmentId}", userId, departmentId).
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:
Line 84:
Unstructured log fields in ChatHub: LogException receives userId and departmentId via string interpolation instead of as separate structured parameters, preventing searchable filtering in SIEM tools. Pass identifiers as structured fields, e.g. Resgrid.Framework.Logging.LogException(ex, "ChatHub presence-offline broadcast failed for user {UserId} in department {DepartmentId}", userId, departmentId).
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| try | ||
| { | ||
| await Clients.Group($"chatdept:{departmentId}").SendAsync("chatPresenceChanged", userId, false); |
There was a problem hiding this comment.
Duplicated inline string literal in ChatHub: the SignalR client-method name "chatPresenceChanged" is hardcoded in SendAsync. Extract it to a constant such as private const string ChatPresenceChangedEvent = "chatPresenceChanged" and reference it in both SendAsync and the client-side handler registration.
Kody rule violation: Centralize string constants
Prompt for LLM
File Web/Resgrid.Web.Eventing/Hubs/ChatHub.cs:
Line 78:
Duplicated inline string literal in ChatHub: the SignalR client-method name "chatPresenceChanged" is hardcoded in SendAsync. Extract it to a constant such as private const string ChatPresenceChangedEvent = "chatPresenceChanged" and reference it in both SendAsync and the client-side handler registration.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return mfaError; | ||
|
|
||
| // Brute-force guard: cap TOTP verification attempts per user per window before we ever verify. | ||
| var attempts = await _cacheProvider.IncrementAsync(GetMfaVerifyRateLimitCacheKey(user.Id), MfaVerifyRateLimitWindow); |
There was a problem hiding this comment.
Unhandled exception in MFA rate-limiting: IncrementAsync against the distributed cache can throw on cache unavailability, surfacing as a raw 500 before the rate-limit decision. Wrap the call in try/catch, log structured context (op:'mfa_rate_limit', userId), and return an application-level result reflecting an explicit fail-open or fail-closed posture. Also found at ChatModerationController.cs:632, ChatModerationController.cs:641, DepartmentsService.cs:782.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ChatModerationController.cs:
Line 618:
Unhandled exception in MFA rate-limiting: IncrementAsync against the distributed cache can throw on cache unavailability, surfacing as a raw 500 before the rate-limit decision. Wrap the call in try/catch, log structured context (op:'mfa_rate_limit', userId), and return an application-level result reflecting an explicit fail-open or fail-closed posture. Also found at ChatModerationController.cs:632, ChatModerationController.cs:641, DepartmentsService.cs:782.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| public static void LogError(Exception exception, string extraMessage = "", string correlationId = "", | ||
| [CallerFilePath] string callerFilePath = "", [CallerMemberName] string callerMemberName = "", [CallerLineNumber] int callerLineNumber = 0) | ||
| { | ||
| Initialize(null); | ||
| string msgToLog = BuildExceptionMessage(exception, extraMessage, correlationId, callerFilePath, callerMemberName, callerLineNumber); | ||
|
|
||
| if (_logger != null) | ||
| _logger.Error(exception, msgToLog); | ||
|
|
||
| Console.WriteLine(exception.ToString() + $" {extraMessage}"); | ||
| } |
There was a problem hiding this comment.
Duplicate statement sequence in the LogError method body, nearly identical to LogException with only the log level differing (Error vs Fatal). Rule 14 requires extracting this shared logic into a reusable helper such as LogAtLevel that both methods delegate to.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Core/Resgrid.Framework/Logging.cs:
Line 82 to 92:
Duplicate statement sequence in the LogError method body, nearly identical to LogException with only the log level differing (Error vs Fatal). Rule 14 requires extracting this shared logic into a reusable helper such as LogAtLevel that both methods delegate to.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (_logger != null) | ||
| _logger.Error(exception, msgToLog); | ||
|
|
||
| Console.WriteLine(exception.ToString() + $" {extraMessage}"); |
There was a problem hiding this comment.
String concatenation with the + operator in Console.WriteLine instead of interpolation. Rule 103 favors a single interpolated expression such as Console.WriteLine($"{exception} {extraMessage}") for readability and fewer errors.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File Core/Resgrid.Framework/Logging.cs:
Line 91:
String concatenation with the + operator in Console.WriteLine instead of interpolation. Rule 103 favors a single interpolated expression such as Console.WriteLine($"{exception} {extraMessage}") for readability and fewer errors.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| catch (Exception ex) | ||
| { | ||
| // Best-effort and explicitly non-fatal to the lane save — log at Error, not Fatal. | ||
| Logging.LogError(ex, "Best-effort chat lane channel provisioning failed after lane save."); |
There was a problem hiding this comment.
Missing structured context fields in the error log. Rule [3] requires error logs to include the operation name and relevant identifiers (node.IncidentCommandId, node.DepartmentId, node.CallId) as structured fields rather than a plain message string.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Core/Resgrid.Services/IncidentCommandService.cs:
Line 1487:
Missing structured context fields in the error log. Rule [3] requires error logs to include the operation name and relevant identifiers (node.IncidentCommandId, node.DepartmentId, node.CallId) as structured fields rather than a plain message string.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
3 participants
Pull Request Description
RG-T117: Realtime Chat System and In-App Chatbot
This pull request introduces a comprehensive real-time chat system and integrates the existing chatbot into a web-based chat interface for the Resgrid platform.
Chat System
The core feature is a full-featured real-time chat system supporting:
Chatbot Web Integration
The existing SMS-based chatbot is now accessible as an in-app AI assistant conversation channel. A guard-railed conversational LLM fallback answers informational questions when no intent matches, while operational commands remain exclusively with the deterministic handlers. Bot replies are persisted into a per-user chatbot channel and delivered over SignalR.
Moderation and Administration
Department admins gain a moderation console with flag review, message deletion, user muting/banning, channel locking, per-department chat settings (retention, content toggles), and transcript export (JSON/CSV/ZIP) for records requests.
Infrastructure
Chat.Systemfeature flag (seeded off) gates the entire system for staged rollout