Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (47)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
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:
|
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
| 6. EXCEPTION SWALLOWING — Catch blocks that silently discard errors? | ||
| → Empty catch, catch with only a log, catch(Exception) without rethrow | ||
|
|
||
| 7. ASYNC DEADLOCKS — .Result, .Wait(), .GetAwaiter().GetResult()? |
There was a problem hiding this comment.
Async deadlock risk identified because blocking async methods using .Result or .Wait() prevents efficient asynchronous execution. Use await instead of blocking calls to ensure proper async behavior.
Kody rule violation: Avoid Blocking Calls to Async Methods
Prompt for LLM
File .agents/skills/80-20-review/SKILL.md:
Line 175:
Async deadlock risk identified because blocking async methods using `.Result` or `.Wait()` prevents efficient asynchronous execution. Use `await` instead of blocking calls to ensure proper async behavior.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| 6. EXCEPTION SWALLOWING — Catch blocks that silently discard errors? | ||
| → Empty catch, catch with only a log, catch(Exception) without rethrow | ||
|
|
||
| 7. ASYNC DEADLOCKS — .Result, .Wait(), .GetAwaiter().GetResult()? |
There was a problem hiding this comment.
Async deadlock risk identified due to blocking calls using .Result, .Wait(), and .GetAwaiter().GetResult(). Await Tasks properly end-to-end and configure awaits appropriately to prevent thread starvation.
Kody rule violation: Await async operations properly
Prompt for LLM
File .agents/skills/80-20-review/SKILL.md:
Line 175:
Async deadlock risk identified due to blocking calls using `.Result`, `.Wait()`, and `.GetAwaiter().GetResult()`. Await Tasks properly end-to-end and configure awaits appropriately to prevent thread starvation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| internal class Handler(AppDbContext db, HybridCache cache) | ||
| { | ||
| public async Task<Response?> Handle(Query query, CancellationToken ct) |
There was a problem hiding this comment.
Missing documentation violation identified on the async method Handle. Add XML doc comments to describe the return type, cancellation behavior, and possible exceptions.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File .agents/skills/caching/SKILL.md:
Line 52:
Missing documentation violation identified on the async method `Handle`. Add XML doc comments to describe the return type, cancellation behavior, and possible exceptions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| services: | ||
| postgres: | ||
| image: postgres:17 |
There was a problem hiding this comment.
Container image drift risk identified because the configuration uses mutable tags like postgres:17. Pin production workloads using immutable image digests such as repo@sha256:... to prevent unexpected breaking changes.
Kody rule violation: Pin container images by digest in Helm/K8s
Prompt for LLM
File .agents/skills/ci-cd/SKILL.md:
Line 46:
Container image drift risk identified because the configuration uses mutable tags like `postgres:17`. Pin production workloads using immutable image digests such as `repo@sha256:...` to prevent unexpected breaking changes.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| var order = Order.Create(request, clock.GetUtcNow()); | ||
| db.Orders.Add(order); | ||
| await db.SaveChangesAsync(ct); |
There was a problem hiding this comment.
Unhandled database exception risk identified because the external call await db.SaveChangesAsync(ct) lacks a try/catch block. Wrap the call in a try/catch(DbUpdateException) to log structured context and map the error to a domain Result.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File .agents/skills/dependency-injection/SKILL.md:
Line 68:
Unhandled database exception risk identified because the external call `await db.SaveChangesAsync(ct)` lacks a try/catch block. Wrap the call in a `try/catch(DbUpdateException)` to log structured context and map the error to a domain Result.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ```csharp | ||
| // BAD — silently swallowing | ||
| try { await ProcessOrder(order); } | ||
| catch (Exception) { /* ignore */ } |
There was a problem hiding this comment.
Silent exception swallowing violation identified in the catch block. Log the exception with context using logger.LogError(ex, "ProcessOrder failed for {OrderId}", order.Id) and rethrow it, or handle it meaningfully.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File .agents/skills/error-handling/SKILL.md:
Line 233:
Silent exception swallowing violation identified in the catch block. Log the exception with context using `logger.LogError(ex, "ProcessOrder failed for {OrderId}", order.Id)` and rethrow it, or handle it meaningfully.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| ```csharp | ||
| // BAD — logging credentials | ||
| logger.LogInformation("User logged in: {Email} with password {Password}", email, password); |
There was a problem hiding this comment.
Credential logging violation identified because the statement outputs full passwords. Remove the password parameter or replace it with a hashed or tokenized representation to protect secrets.
Kody rule violation: Mask PII and secrets in logs
Prompt for LLM
File .agents/skills/logging/SKILL.md:
Line 137:
Credential logging violation identified because the statement outputs full passwords. Remove the password parameter or replace it with a hashed or tokenized representation to protect secrets.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .WithSummary("Create a new order") | ||
| .Produces<OrderResponse>(StatusCodes.Status201Created) | ||
| .ProducesValidationProblem() | ||
| .RequireAuthorization(); |
There was a problem hiding this comment.
Broken access control risk identified because the bare .RequireAuthorization() only enforces authentication and not role- or scope-based access control. Pass a specific deny-by-default policy name such as .RequireAuthorization("OrderWrite") to enforce proper RBAC.
Kody rule violation: Implement RBAC with least privilege and deny-by-default
Prompt for LLM
File .agents/skills/minimal-api/SKILL.md:
Line 76:
Broken access control risk identified because the bare `.RequireAuthorization()` only enforces authentication and not role- or scope-based access control. Pass a specific deny-by-default policy name such as `.RequireAuthorization("OrderWrite")` to enforce proper RBAC.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (order is { Customer: { Address: { Country: { Code: "US" } } } }) | ||
|
|
||
| // GOOD — extract to a clear method or use sequential checks | ||
| if (order.Customer.Address.Country.Code == "US") |
There was a problem hiding this comment.
NullReferenceException risk identified due to deep dereferencing of order.Customer.Address.Country.Code without null guards. Use null-conditional access (order?.Customer?.Address?.Country?.Code == "US") or a pattern matching statement to satisfy the null-check requirement.
Kody rule violation: Add null checks to prevent NullReferenceException
Prompt for LLM
File .agents/skills/modern-csharp/SKILL.md:
Line 153:
NullReferenceException risk identified due to deep dereferencing of `order.Customer.Address.Country.Code` without null guards. Use null-conditional access (`order?.Customer?.Address?.Country?.Code == "US"`) or a pattern matching statement to satisfy the null-check requirement.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // GOOD — extension members (C# 14) | ||
| public extension OrderExtensions for Order | ||
| { | ||
| public decimal TotalWithTax => Total * 1.2m; |
There was a problem hiding this comment.
Magic number violation identified where the tax rate 1.2m is buried inline in the TotalWithTax calculation. Extract the multiplier to a named constant like private const decimal TaxMultiplier = 1.2m; to improve maintainability.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File .agents/skills/modern-csharp/SKILL.md:
Line 116:
Magic number violation identified where the tax rate `1.2m` is buried inline in the `TotalWithTax` calculation. Extract the multiplier to a named constant like `private const decimal TaxMultiplier = 1.2m;` to improve maintainability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (order is { Customer: { Address: { Country: { Code: "US" } } } }) | ||
|
|
||
| // GOOD — extract to a clear method or use sequential checks | ||
| if (order.Customer.Address.Country.Code == "US") |
There was a problem hiding this comment.
Magic string violation identified in the comparison against the raw string literal "US". Replace the literal with a centralized enum or named constant like CountryCode.US to prevent typos.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File .agents/skills/modern-csharp/SKILL.md:
Line 153:
Magic string violation identified in the comparison against the raw string literal `"US"`. Replace the literal with a centralized enum or named constant like `CountryCode.US` to prevent typos.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| logger.LogInformation("Login: {Email} with password {Password}", email, password); | ||
|
|
||
| // GOOD — never log secrets, passwords, tokens, or PII | ||
| logger.LogInformation("Login: {Email}", email); |
There was a problem hiding this comment.
GDPR compliance violation identified because raw email personal data is logged without lawful-basis or purpose metadata. Emit a hashed identifier instead and attach GDPR context to the diagnostic log.
Kody rule violation: Redact PII in logs and metrics by default
Prompt for LLM
File .agents/skills/serilog/SKILL.md:
Line 236:
GDPR compliance violation identified because raw email personal data is logged without lawful-basis or purpose metadata. Emit a hashed identifier instead and attach GDPR context to the diagnostic log.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| public new async Task DisposeAsync() | ||
| { | ||
| await _postgres.DisposeAsync(); |
There was a problem hiding this comment.
Resource leak risk identified because an unhandled rejection in _postgres.DisposeAsync() skips base.DisposeAsync() on line 100. Wrap the awaited operation in a try/finally block to guarantee base-class cleanup executes during container teardown.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File .agents/skills/testing/SKILL.md:
Line 99:
Resource leak risk identified because an unhandled rejection in `_postgres.DisposeAsync()` skips `base.DisposeAsync()` on line 100. Wrap the awaited operation in a try/finally block to guarantee base-class cleanup executes during container teardown.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var failures = validators | ||
| .Select(v => v.Validate(context)) | ||
| .SelectMany(r => r.Errors) | ||
| .Where(f => f is not null) | ||
| .ToList(); |
There was a problem hiding this comment.
Readability violation identified because the single-expression LINQ chain combines four operations (Select, SelectMany, Where, ToList). Split the operations into intermediate variables to improve debugging and code clarity.
Kody rule violation: Limit Lengthy LINQ Chains
Prompt for LLM
File .agents/skills/vertical-slice/SKILL.md:
Line 227 to 231:
Readability violation identified because the single-expression LINQ chain combines four operations (`Select`, `SelectMany`, `Where`, `ToList`). Split the operations into intermediate variables to improve debugging and code clarity.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| public void Map(IEndpointRouteBuilder app) | ||
| { | ||
| var group = app.MapGroup("/api/orders").WithTags("Orders"); |
There was a problem hiding this comment.
Route drift risk identified due to duplicating the inline string literal "/api/orders" across multiple locations (lines 99, 105, 178). Extract the route path to a constant such as const string OrdersRoute = "/api/orders"; to centralize the configuration.
Kody rule violation: Centralize string constants
Prompt for LLM
File .agents/skills/vertical-slice/SKILL.md:
Line 99:
Route drift risk identified due to duplicating the inline string literal `"/api/orders"` across multiple locations (lines 99, 105, 178). Extract the route path to a constant such as `const string OrdersRoute = "/api/orders";` to centralize the configuration.
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}
2 participants
PR Description
This pull request adds a comprehensive set of AI agent skills (prompt-based guidance documents) to the repository under the
.agents/skills/directory. These are structured markdown files that provide specialized, context-aware instructions for an AI coding assistant working within the Resgrid/Core codebase.Changes Summary
47 new skill files were added, each defining a specific area of expertise:
Architecture & Design:
architecture-advisor,vertical-slice,clean-architecture,ddd,project-structure,minimal-api,api-versioningDevelopment Patterns:
ef-core,dependency-injection,caching,messaging,error-handling,logging,serilog,resilience,httpclient-factory,authentication,configuration,modern-csharp,scaffolding,migration-workflowInfrastructure & Deployment:
docker,container-publish,ci-cd,aspire,openapi,scalar,opentelemetryQuality & Review:
code-review-workflow,80-20-review,verification-loop,de-sloppify,security-scan,health-check,testingAgent Workflow & Session Management:
session-management,wrap-up-ritual,autonomous-loops,workflow-mastery,context-discipline,model-selectionself-correction-loop,instinct-system,convention-learner,learning-log,split-memoryproject-setupEach skill file follows a consistent structure with core principles, patterns, anti-patterns, and decision guides tailored for .NET 10 / C# 14 development.
Functional Impact
No application code was modified. This commit exclusively introduces documentation/configuration for an AI development assistant, establishing standardized guidance for code generation, review, testing, and project maintenance workflows within the Resgrid/Core repository.