feat: add C# language support (rebased #652)#676
Closed
Conversation
Contributor
|
Contributor
📝 WalkthroughWalkthroughAdds first-class C# /.NET 8+ support: five new rule documents under Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Important Merge conflicts detected (Beta)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
Analysis CompleteGenerated ECC bundle from 500 commits | Confidence: 100% View Pull Request #677Repository Profile
Detected Workflows (9)
Generated Instincts (16)
After merging, import with: Files
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/csharp-security/SKILL.md">
<violation number="1" location="skills/csharp-security/SKILL.md:275">
P2: Guard `UseHsts()` behind a non-development environment check; the current example contradicts its own "production only" note and can misconfigure local/dev environments.</violation>
</file>
<file name="rules/csharp/coding-style.md">
<violation number="1" location="rules/csharp/coding-style.md:14">
P2: Avoid recommending `<LangVersion>latest</LangVersion>`; it can make builds non-reproducible across environments and permit unsupported language/runtime combinations for the target framework.</violation>
</file>
<file name="skills/csharp-patterns/SKILL.md">
<violation number="1" location="skills/csharp-patterns/SKILL.md:199">
P2: Dispose `HttpResponseMessage` in the async HTTP example and check for non-success status codes before reading content.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+275
to
+276
| app.UseHsts(); | ||
| app.UseHttpsRedirection(); |
Contributor
There was a problem hiding this comment.
P2: Guard UseHsts() behind a non-development environment check; the current example contradicts its own "production only" note and can misconfigure local/dev environments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/csharp-security/SKILL.md, line 275:
<comment>Guard `UseHsts()` behind a non-development environment check; the current example contradicts its own "production only" note and can misconfigure local/dev environments.</comment>
<file context>
@@ -0,0 +1,352 @@
+ options.IncludeSubDomains = true;
+ options.Preload = true;
+});
+app.UseHsts();
+app.UseHttpsRedirection();
+```
</file context>
Suggested change
| app.UseHsts(); | |
| app.UseHttpsRedirection(); | |
| if (!app.Environment.IsDevelopment()) | |
| { | |
| app.UseHsts(); | |
| } | |
| app.UseHttpsRedirection(); |
| public async Task<byte[]> DownloadAsync(string url, CancellationToken ct) | ||
| { | ||
| using var client = httpClientFactory.CreateClient(); | ||
| var response = await client.GetAsync(url, ct).ConfigureAwait(false); |
Contributor
There was a problem hiding this comment.
P2: Dispose HttpResponseMessage in the async HTTP example and check for non-success status codes before reading content.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/csharp-patterns/SKILL.md, line 199:
<comment>Dispose `HttpResponseMessage` in the async HTTP example and check for non-success status codes before reading content.</comment>
<file context>
@@ -0,0 +1,350 @@
+ public async Task<byte[]> DownloadAsync(string url, CancellationToken ct)
+ {
+ using var client = httpClientFactory.CreateClient();
+ var response = await client.GetAsync(url, ct).ConfigureAwait(false);
+ return await response.Content.ReadAsByteArrayAsync(ct).ConfigureAwait(false);
+ }
</file context>
Contributor
Greptile SummaryThis PR rebases community contribution #652 to add C# / .NET 8+ language support: five rule files ( Key concerns:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Install script / manual copy] --> B[rules/common/]
A --> C[rules/csharp/]
C --> C1[coding-style.md]
C --> C2[patterns.md]
C --> C3[security.md]
C --> C4[testing.md]
C --> C5[hooks.md]
C1 -->|extends| B
C2 -->|extends| B
C3 -->|extends| B
C4 -->|extends| B
C5 -->|extends| B
C2 -->|references| S1[skills/csharp-patterns]
C3 -->|references| S2[skills/csharp-security]
C4 -->|references| S3[skills/csharp-testing]
C1 -->|references| S1
S1 --- S4[skills/csharp-data-access]
Last reviewed commit: "fix: address PR revi..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b19912a839
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+175
to
+176
| ./install.sh typescript # or python or golang or swift or php or csharp | ||
| # ./install.sh typescript python golang swift php csharp |
There was a problem hiding this comment.
csharp install example
For users following the new quickstart, ./install.sh csharp still cannot be a valid legacy install target: scripts/lib/install-manifests.js:37-57 never maps csharp, and resolveLegacyCompatibilitySelection() rejects any unknown language at scripts/lib/install-manifests.js:243-255. As written, the first documented onboarding path for C# users aborts before any install plan is created.
Useful? React with 👍 / 👎.
Comment on lines
+360
to
+363
| | |-- csharp-patterns/ # C# 12 / .NET 8+ idioms and best practices (NEW) | ||
| | |-- csharp-security/ # C# / ASP.NET Core security patterns (NEW) | ||
| | |-- csharp-testing/ # C# TDD with xUnit, NSubstitute, Coverlet (NEW) | ||
| | |-- csharp-data-access/ # EF Core and Dapper data access patterns (NEW) |
There was a problem hiding this comment.
These four new skill directories are only added to the repo tree here; none of the install modules reference them. The manifest-backed installers copy only paths declared in manifests/install-modules.json, but framework-language (102-139), database (158-166), and security (215-226) omit every skills/csharp-* path, so even a --profile full/developer install ships zero C# skills. Users installing ECC into Claude/Cursor/Codex therefore do not actually receive the new C# guidance this change advertises.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 175-176: Update the README example or the language alias mapping
to avoid the failing "Unknown legacy language: csharp" error: either remove
'csharp' from the install.sh example in README.md (the line showing
"./install.sh typescript # or python or golang or swift or php or csharp") or
add a 'csharp' entry to the LEGACY_LANGUAGE_ALIAS_TO_CANONICAL mapping in
scripts/lib/install-manifests.js (so the validation logic that checks legacy
aliases recognizes 'csharp'); ensure the mapping key matches the legacy alias
'csharp' and maps to the correct canonical language name used elsewhere so the
validation code that builds allowed languages will pass.
In `@rules/csharp/patterns.md`:
- Line 10: Replace the phrase "C# specific content" with the hyphenated form
"C#-specific content" in the document (look for the exact string "C# specific
content" to locate it) so the line reads "This file extends
[common/patterns.md](../common/patterns.md) with C#-specific content."
In `@skills/csharp-data-access/SKILL.md`:
- Around line 20-410: The SKILL.md is missing the required "How It Works" and
"Examples" sections; add two new Markdown headings "How It Works" (after the
existing "When to Use" heading) and "Examples" and reorganize content so
explanatory text (concepts, best practices, lifecycle explanations) goes under
"How It Works" while concrete code blocks and runnable snippets (DbContext
Setup, DbContext Lifetime, Entity Configuration, Query Optimization examples,
Pagination, Migrations, Transactions, Dapper, Repository Pattern, Auditing) are
grouped under "Examples"; ensure headings are top-level and preserve existing
code blocks and headings like "DbContext Setup", "Entity Configuration", "Query
Optimization", "Migrations", "Transactions", "Dapper", and "Repository Pattern"
by moving them into the new "Examples" or "How It Works" sections appropriately.
- Around line 23-25: The comment says "register with connection pooling" but the
code uses AddDbContext, which does not enable pooling; either update the comment
to correctly state that AppDbContext is registered without pooling, or switch
the registration to AddDbContextPool<AppDbContext>(...) to enable pooling (keep
the same options.UseNpgsql(...) configuration). Locate the registration call
(AddDbContext/AddDbContextPool) in Program.cs where AppDbContext is being
configured and apply the chosen change so the comment and implementation match.
In `@skills/csharp-security/SKILL.md`:
- Around line 20-352: The skill markdown is missing the required "How It Works"
and "Examples" sections; update SKILL.md to add a "How It Works" section
explaining the core concepts and flows (e.g., Identity + JWT validation,
policy-based auth, input validation, SQL/XSS/CSRF mitigations) and an "Examples"
section with concise usage snippets or scenarios that reference existing
examples (e.g., the Program.cs Identity/JWT blocks,
CreateUserRequest/FluentValidation snippets, SecurityHeadersMiddleware); ensure
the file follows the required skill format with "When to Use", "How It Works",
and "Examples" headings in Markdown and place them logically near the top-level
sections so reviewers can find them easily.
- Around line 268-276: HSTS is configured but app.UseHsts() is called
unconditionally; wrap the HSTS setup and call so it only runs in non-development
environments (use the application environment check, e.g.,
app.Environment.IsDevelopment()/IWebHostEnvironment.IsDevelopment) so that
builder.Services.AddHsts(...) and app.UseHsts() are executed only when not in
development while leaving app.UseHttpsRedirection() as appropriate.
In `@skills/csharp-testing/SKILL.md`:
- Around line 19-411: The SKILL.md currently has detailed content (sections like
"TDD Workflow", "xUnit Fundamentals", "FluentAssertions", "Integration Testing")
but lacks explicit "How It Works" and "Examples" headings required by our skill
format; update the document by adding a "How It Works" section that summarizes
the workflow and testing principles (extract from TDD Workflow, xUnit
Fundamentals, IAsyncLifetime, Arrange-Act-Assert, Best Practices) and an
"Examples" section that collects runnable examples (the RED-GREEN-REFACTOR
snippet, xUnit Facts/Theories examples, WebApplicationFactory and TestContainers
snippets) while keeping or adding a "When to Use" header near the top; ensure
headings are clear Markdown headers and move or duplicate relevant code blocks
under the new sections so the file follows the "When to Use / How It Works /
Examples" structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf6d2da7-e4de-49ce-b010-b4feae191829
📒 Files selected for processing (12)
AGENTS.mdREADME.mdrules/README.mdrules/csharp/coding-style.mdrules/csharp/hooks.mdrules/csharp/patterns.mdrules/csharp/security.mdrules/csharp/testing.mdskills/csharp-data-access/SKILL.mdskills/csharp-patterns/SKILL.mdskills/csharp-security/SKILL.mdskills/csharp-testing/SKILL.md
Comment on lines
+175
to
+176
| ./install.sh typescript # or python or golang or swift or php or csharp | ||
| # ./install.sh typescript python golang swift php csharp |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if install.sh accepts 'csharp' as a language argument
# Test 1: Check LEGACY_LANGUAGE_ALIAS_TO_CANONICAL in install-manifests.js
echo "=== Checking install-manifests.js for 'csharp' support ==="
rg -n "LEGACY_LANGUAGE_ALIAS_TO_CANONICAL" scripts/lib/install-manifests.js -A 15
# Test 2: Check if there's alternative logic that handles 'csharp'
echo -e "\n=== Searching for 'csharp' language handling ==="
rg -n "'csharp'|\"csharp\"" scripts/ --type js
# Test 3: Check rules/csharp directory exists
echo -e "\n=== Verifying rules/csharp/ directory exists ==="
fd -t d "^csharp$" rules/Repository: affaan-m/everything-claude-code
Length of output: 2637
🏁 Script executed:
# Check install.sh implementation
echo "=== Reading install.sh script ==="
cat -n install.sh | head -50
echo -e "\n=== Searching for how install.sh calls install-manifests.js ==="
rg -n "install-manifests" install.sh -A 5 -B 5
# Check if there's a modern (non-legacy) language support mechanism
echo -e "\n=== Checking for non-legacy language handling ==="
rg -n "csharp" scripts/lib/install-manifests.js -B 3 -A 3
# Check what happens at the validation step
echo -e "\n=== Checking validateLegacyLanguages function ==="
rg -n "function validateLegacyLanguages" scripts/lib/install-manifests.js -A 30Repository: affaan-m/everything-claude-code
Length of output: 980
🏁 Script executed:
# Check install-apply.js implementation
echo "=== Reading install-apply.js (first 80 lines) ==="
head -80 scripts/install-apply.js
# Search for language handling in install-apply.js
echo -e "\n=== Searching for language argument handling ==="
rg -n "csharp\|language\|argv" scripts/install-apply.js | head -30
# Check the full install-apply.js size and structure
echo -e "\n=== install-apply.js file info ==="
wc -l scripts/install-apply.jsRepository: affaan-m/everything-claude-code
Length of output: 3544
🏁 Script executed:
# Check install/request.js to understand language validation
echo "=== Reading install/request.js to check language validation ==="
wc -l scripts/lib/install/request.js
head -100 scripts/lib/install/request.js
# Search for how legacy language validation is done
echo -e "\n=== Searching for validateLegacyLanguages calls ==="
rg -n "validateLegacyLanguages" scripts/ --type js -A 10Repository: affaan-m/everything-claude-code
Length of output: 3607
🏁 Script executed:
# Find validateLegacyLanguages function definition and all its usages
echo "=== Searching for validateLegacyLanguages definition and usage ==="
rg -n "validateLegacyLanguages" scripts/ --type js -B 2 -A 5
# Read the rest of install/request.js
echo -e "\n=== Reading the rest of install/request.js (after line 100) ==="
tail -20 scripts/lib/install/request.js
# Search for all csharp references in the entire scripts directory
echo -e "\n=== Searching for 'csharp' in all scripts ==="
rg -n "csharp" scripts/ --type jsRepository: affaan-m/everything-claude-code
Length of output: 769
🏁 Script executed:
# Read full install-apply.js to see complete flow
echo "=== Reading full install-apply.js ==="
cat -n scripts/install-apply.js
# Search for where legacyLanguages are used/validated
echo -e "\n=== Searching for legacyLanguages processing ==="
rg -n "legacyLanguages" scripts/ --type js -B 2 -A 5Repository: affaan-m/everything-claude-code
Length of output: 17363
🏁 Script executed:
# Identify which function contains the validation logic for legacy languages
echo "=== Finding function that validates legacy languages ==="
rg -n "Unknown legacy language" scripts/lib/install-manifests.js -B 20 | head -40Repository: affaan-m/everything-claude-code
Length of output: 1280
Update README.md to remove 'csharp' from the install.sh example or add csharp support to the language alias mapping.
The documentation at lines 175-176 instructs users to run ./install.sh csharp, but this will fail because csharp is not in the LEGACY_LANGUAGE_ALIAS_TO_CANONICAL mapping in scripts/lib/install-manifests.js (lines 37-48). When users attempt to install csharp via install.sh, the validation logic (lines 243-256) will throw an error: "Unknown legacy language: csharp. Expected one of go, golang, java, javascript, kotlin, perl, php, python, swift, typescript."
Either remove csharp from the documented command examples or add it to the supported language aliases in scripts/lib/install-manifests.js.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 175 - 176, Update the README example or the language
alias mapping to avoid the failing "Unknown legacy language: csharp" error:
either remove 'csharp' from the install.sh example in README.md (the line
showing "./install.sh typescript # or python or golang or swift or php or
csharp") or add a 'csharp' entry to the LEGACY_LANGUAGE_ALIAS_TO_CANONICAL
mapping in scripts/lib/install-manifests.js (so the validation logic that checks
legacy aliases recognizes 'csharp'); ensure the mapping key matches the legacy
alias 'csharp' and maps to the correct canonical language name used elsewhere so
the validation code that builds allowed languages will pass.
| --- | ||
| # C# Patterns | ||
|
|
||
| > This file extends [common/patterns.md](../common/patterns.md) with C# specific content. |
Contributor
There was a problem hiding this comment.
Use hyphenated compound adjective for clarity.
Line 10 should use “C#-specific content” instead of “C# specific content.”
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...terns.md](../common/patterns.md) with C# specific content. ## Repository Pattern...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/csharp/patterns.md` at line 10, Replace the phrase "C# specific
content" with the hyphenated form "C#-specific content" in the document (look
for the exact string "C# specific content" to locate it) so the line reads "This
file extends [common/patterns.md](../common/patterns.md) with C#-specific
content."
Comment on lines
+20
to
+410
| ## DbContext Setup | ||
|
|
||
| ```csharp | ||
| // Program.cs — register with connection pooling | ||
| builder.Services.AddDbContext<AppDbContext>(options => | ||
| options.UseNpgsql(builder.Configuration.GetConnectionString("Default"), | ||
| npgsql => | ||
| { | ||
| npgsql.EnableRetryOnFailure(maxRetryCount: 3); | ||
| npgsql.CommandTimeout(30); | ||
| npgsql.MigrationsHistoryTable("__ef_migrations"); | ||
| })); | ||
|
|
||
| // AppDbContext.cs | ||
| public class AppDbContext(DbContextOptions<AppDbContext> options) : DbContext(options) | ||
| { | ||
| public DbSet<Order> Orders => Set<Order>(); | ||
| public DbSet<Product> Products => Set<Product>(); | ||
| public DbSet<Customer> Customers => Set<Customer>(); | ||
|
|
||
| protected override void OnModelCreating(ModelBuilder modelBuilder) | ||
| { | ||
| modelBuilder.ApplyConfigurationsFromAssembly(typeof(AppDbContext).Assembly); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### DbContext Lifetime | ||
|
|
||
| ```csharp | ||
| // Scoped (default) — one per HTTP request, recommended for web apps | ||
| builder.Services.AddDbContext<AppDbContext>(); | ||
|
|
||
| // Pooled — reuses DbContext instances, reduces allocation overhead | ||
| builder.Services.AddDbContextPool<AppDbContext>(options => | ||
| options.UseNpgsql(connectionString), poolSize: 128); | ||
|
|
||
| // Factory — for background services and multi-tenant scenarios | ||
| builder.Services.AddDbContextFactory<AppDbContext>(); | ||
|
|
||
| public class OrderProcessor(IDbContextFactory<AppDbContext> factory) | ||
| { | ||
| public async Task ProcessAsync(CancellationToken ct) | ||
| { | ||
| await using var db = await factory.CreateDbContextAsync(ct); | ||
| // Each call gets a fresh DbContext — safe for long-running services | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Entity Configuration | ||
|
|
||
| ### Fluent API (Preferred over Data Annotations) | ||
|
|
||
| ```csharp | ||
| // Configurations/OrderConfiguration.cs | ||
| public class OrderConfiguration : IEntityTypeConfiguration<Order> | ||
| { | ||
| public void Configure(EntityTypeBuilder<Order> builder) | ||
| { | ||
| builder.ToTable("orders"); | ||
| builder.HasKey(o => o.Id); | ||
|
|
||
| builder.Property(o => o.Id) | ||
| .HasColumnName("id") | ||
| .ValueGeneratedOnAdd(); | ||
|
|
||
| builder.Property(o => o.OrderNumber) | ||
| .HasColumnName("order_number") | ||
| .HasMaxLength(50) | ||
| .IsRequired(); | ||
|
|
||
| builder.Property(o => o.Total) | ||
| .HasColumnName("total") | ||
| .HasPrecision(18, 2); | ||
|
|
||
| builder.Property(o => o.Status) | ||
| .HasColumnName("status") | ||
| .HasConversion<string>() | ||
| .HasMaxLength(20); | ||
|
|
||
| builder.Property(o => o.CreatedAt) | ||
| .HasColumnName("created_at") | ||
| .HasDefaultValueSql("now()"); | ||
|
|
||
| // Indexes | ||
| builder.HasIndex(o => o.OrderNumber).IsUnique(); | ||
| builder.HasIndex(o => o.CustomerId); | ||
| builder.HasIndex(o => new { o.Status, o.CreatedAt }); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Relationships | ||
|
|
||
| ```csharp | ||
| // One-to-Many: Customer has many Orders | ||
| builder.HasOne(o => o.Customer) | ||
| .WithMany(c => c.Orders) | ||
| .HasForeignKey(o => o.CustomerId) | ||
| .OnDelete(DeleteBehavior.Restrict); | ||
|
|
||
| // Many-to-Many: Order has many Products via OrderItems | ||
| builder.HasMany(o => o.Items) | ||
| .WithOne(i => i.Order) | ||
| .HasForeignKey(i => i.OrderId) | ||
| .OnDelete(DeleteBehavior.Cascade); | ||
|
|
||
| // Owned types (value objects stored in same table) | ||
| builder.OwnsOne(o => o.ShippingAddress, address => | ||
| { | ||
| address.Property(a => a.Street).HasColumnName("shipping_street").HasMaxLength(200); | ||
| address.Property(a => a.City).HasColumnName("shipping_city").HasMaxLength(100); | ||
| address.Property(a => a.PostalCode).HasColumnName("shipping_postal_code").HasMaxLength(20); | ||
| }); | ||
| ``` | ||
|
|
||
| ## Query Optimization | ||
|
|
||
| ### Avoiding N+1 Queries | ||
|
|
||
| ```csharp | ||
| // Bad: N+1 — loads orders, then 1 query per customer | ||
| var orders = await db.Orders.ToListAsync(ct); | ||
| foreach (var order in orders) | ||
| Console.WriteLine(order.Customer.Name); // Lazy load = N extra queries | ||
|
|
||
| // Good: Eager loading with Include | ||
| var orders = await db.Orders | ||
| .Include(o => o.Customer) | ||
| .Include(o => o.Items) | ||
| .ThenInclude(i => i.Product) | ||
| .Where(o => o.Status == OrderStatus.Active) | ||
| .ToListAsync(ct); | ||
|
|
||
| // Good: Split query for large includes (avoids cartesian explosion) | ||
| var orders = await db.Orders | ||
| .Include(o => o.Items) | ||
| .AsSplitQuery() | ||
| .ToListAsync(ct); | ||
| ``` | ||
|
|
||
| ### Projections (Select Only What You Need) | ||
|
|
||
| ```csharp | ||
| // Good: Project to DTO — only selects needed columns | ||
| var orderSummaries = await db.Orders | ||
| .Where(o => o.CreatedAt >= cutoff) | ||
| .Select(o => new OrderSummaryDto | ||
| { | ||
| Id = o.Id, | ||
| OrderNumber = o.OrderNumber, | ||
| CustomerName = o.Customer.Name, | ||
| Total = o.Total, | ||
| ItemCount = o.Items.Count, | ||
| }) | ||
| .OrderByDescending(o => o.Total) | ||
| .Take(50) | ||
| .ToListAsync(ct); | ||
|
|
||
| // Good: AsNoTracking for read-only queries (reduces memory overhead) | ||
| var products = await db.Products | ||
| .AsNoTracking() | ||
| .Where(p => p.Category == category) | ||
| .ToListAsync(ct); | ||
| ``` | ||
|
|
||
| ### Pagination | ||
|
|
||
| ```csharp | ||
| public async Task<PagedResult<OrderDto>> GetOrdersAsync(int page, int size, CancellationToken ct) | ||
| { | ||
| var query = db.Orders.AsNoTracking().Where(o => o.Status == OrderStatus.Active); | ||
|
|
||
| var total = await query.CountAsync(ct); | ||
| var items = await query | ||
| .OrderByDescending(o => o.CreatedAt) | ||
| .Skip((page - 1) * size) | ||
| .Take(size) | ||
| .Select(o => new OrderDto(o.Id, o.OrderNumber, o.Total)) | ||
| .ToListAsync(ct); | ||
|
|
||
| return new PagedResult<OrderDto>(items, total, page, size); | ||
| } | ||
|
|
||
| public record PagedResult<T>(IReadOnlyList<T> Items, int Total, int Page, int Size); | ||
| ``` | ||
|
|
||
| ## Migrations | ||
|
|
||
| ```bash | ||
| # Create a migration | ||
| dotnet ef migrations add AddOrdersTable | ||
|
|
||
| # Apply migrations | ||
| dotnet ef database update | ||
|
|
||
| # Generate SQL script (for production deployments) | ||
| dotnet ef migrations script --idempotent -o migrate.sql | ||
|
|
||
| # Remove last migration (if not applied) | ||
| dotnet ef migrations remove | ||
| ``` | ||
|
|
||
| ```csharp | ||
| // Good: Apply migrations at startup (development only) | ||
| if (app.Environment.IsDevelopment()) | ||
| { | ||
| using var scope = app.Services.CreateScope(); | ||
| var db = scope.ServiceProvider.GetRequiredService<AppDbContext>(); | ||
| await db.Database.MigrateAsync(); | ||
| } | ||
|
|
||
| // Production: Use generated SQL scripts or a migration tool | ||
| ``` | ||
|
|
||
| ## Transactions | ||
|
|
||
| ```csharp | ||
| // EF Core: SaveChanges is already transactional | ||
| public async Task PlaceOrderAsync(Order order, CancellationToken ct) | ||
| { | ||
| db.Orders.Add(order); | ||
| foreach (var item in order.Items) | ||
| db.OrderItems.Add(item); | ||
|
|
||
| await db.SaveChangesAsync(ct); // All-or-nothing | ||
| } | ||
|
|
||
| // Explicit transaction for multi-step operations | ||
| public async Task TransferAsync(int fromId, int toId, decimal amount, CancellationToken ct) | ||
| { | ||
| await using var transaction = await db.Database.BeginTransactionAsync(ct); | ||
| try | ||
| { | ||
| var from = await db.Accounts.FindAsync([fromId], ct) | ||
| ?? throw new AccountNotFoundException(fromId); | ||
| var to = await db.Accounts.FindAsync([toId], ct) | ||
| ?? throw new AccountNotFoundException(toId); | ||
|
|
||
| from.Balance -= amount; | ||
| to.Balance += amount; | ||
|
|
||
| await db.SaveChangesAsync(ct); | ||
| await transaction.CommitAsync(ct); | ||
| } | ||
| catch | ||
| { | ||
| await transaction.RollbackAsync(ct); | ||
| throw; | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Dapper (Micro-ORM) | ||
|
|
||
| Use Dapper for performance-critical, read-heavy queries where EF Core overhead matters. | ||
|
|
||
| ```csharp | ||
| // Basic query | ||
| public async Task<Order?> GetOrderAsync(IDbConnection db, int id) | ||
| { | ||
| return await db.QuerySingleOrDefaultAsync<Order>( | ||
| "SELECT id, order_number, total, status FROM orders WHERE id = @Id", | ||
| new { Id = id }); | ||
| } | ||
|
|
||
| // Multi-mapping (join result to multiple objects) | ||
| public async Task<IReadOnlyList<OrderWithCustomer>> GetOrdersWithCustomersAsync(IDbConnection db) | ||
| { | ||
| const string sql = """ | ||
| SELECT o.id, o.order_number, o.total, c.id, c.name, c.email | ||
| FROM orders o | ||
| INNER JOIN customers c ON o.customer_id = c.id | ||
| ORDER BY o.created_at DESC | ||
| LIMIT 100 | ||
| """; | ||
|
|
||
| var result = await db.QueryAsync<OrderWithCustomer, CustomerDto, OrderWithCustomer>( | ||
| sql, | ||
| (order, customer) => order with { Customer = customer }, | ||
| splitOn: "id"); | ||
|
|
||
| return result.ToList().AsReadOnly(); | ||
| } | ||
|
|
||
| // Bulk insert with Dapper | ||
| public async Task BulkInsertAsync(IDbConnection db, IEnumerable<Product> products) | ||
| { | ||
| const string sql = """ | ||
| INSERT INTO products (name, price, category) | ||
| VALUES (@Name, @Price, @Category) | ||
| """; | ||
| await db.ExecuteAsync(sql, products); | ||
| } | ||
| ``` | ||
|
|
||
| ### When to Use EF Core vs Dapper | ||
|
|
||
| | Scenario | Recommendation | | ||
| |---|---| | ||
| | CRUD operations with relationships | EF Core | | ||
| | Complex reporting queries | Dapper | | ||
| | Migrations and schema management | EF Core | | ||
| | Bulk inserts/updates | Dapper | | ||
| | Rapid prototyping | EF Core | | ||
| | Performance-critical read paths | Dapper | | ||
| | Change tracking needed | EF Core | | ||
| | Stored procedure calls | Dapper | | ||
|
|
||
| ## Repository Pattern | ||
|
|
||
| ```csharp | ||
| // Generic repository interface | ||
| public interface IRepository<T> where T : class | ||
| { | ||
| Task<T?> FindByIdAsync(int id, CancellationToken ct = default); | ||
| Task<IReadOnlyList<T>> GetAllAsync(CancellationToken ct = default); | ||
| Task AddAsync(T entity, CancellationToken ct = default); | ||
| void Update(T entity); | ||
| void Remove(T entity); | ||
| } | ||
|
|
||
| // EF Core implementation | ||
| public class EfRepository<T>(AppDbContext db) : IRepository<T> where T : class | ||
| { | ||
| public async Task<T?> FindByIdAsync(int id, CancellationToken ct) | ||
| => await db.Set<T>().FindAsync([id], ct); | ||
|
|
||
| public async Task<IReadOnlyList<T>> GetAllAsync(CancellationToken ct) | ||
| => await db.Set<T>().AsNoTracking().ToListAsync(ct); | ||
|
|
||
| public async Task AddAsync(T entity, CancellationToken ct) | ||
| => await db.Set<T>().AddAsync(entity, ct); | ||
|
|
||
| public void Update(T entity) => db.Set<T>().Update(entity); | ||
| public void Remove(T entity) => db.Set<T>().Remove(entity); | ||
| } | ||
|
|
||
| // Unit of Work | ||
| public interface IUnitOfWork | ||
| { | ||
| IRepository<Order> Orders { get; } | ||
| IRepository<Product> Products { get; } | ||
| Task<int> SaveChangesAsync(CancellationToken ct = default); | ||
| } | ||
|
|
||
| public class UnitOfWork(AppDbContext db) : IUnitOfWork | ||
| { | ||
| public IRepository<Order> Orders { get; } = new EfRepository<Order>(db); | ||
| public IRepository<Product> Products { get; } = new EfRepository<Product>(db); | ||
|
|
||
| public Task<int> SaveChangesAsync(CancellationToken ct) => db.SaveChangesAsync(ct); | ||
| } | ||
| ``` | ||
|
|
||
| ## Auditing | ||
|
|
||
| ```csharp | ||
| // Base entity with audit fields | ||
| public abstract class AuditableEntity | ||
| { | ||
| public DateTime CreatedAt { get; init; } | ||
| public DateTime? UpdatedAt { get; private set; } | ||
|
|
||
| public void MarkUpdated() => UpdatedAt = DateTime.UtcNow; | ||
| } | ||
|
|
||
| // Override SaveChanges to set audit fields automatically | ||
| public override async Task<int> SaveChangesAsync(CancellationToken ct = default) | ||
| { | ||
| foreach (var entry in ChangeTracker.Entries<AuditableEntity>()) | ||
| { | ||
| if (entry.State == EntityState.Added) | ||
| entry.Property(nameof(AuditableEntity.CreatedAt)).CurrentValue = DateTime.UtcNow; | ||
|
|
||
| if (entry.State == EntityState.Modified) | ||
| entry.Entity.MarkUpdated(); | ||
| } | ||
|
|
||
| return await base.SaveChangesAsync(ct); | ||
| } | ||
| ``` | ||
|
|
||
| ## Best Practices | ||
|
|
||
| **DO:** Use `AsNoTracking()` for read-only queries | Use projections (`Select`) over full entity loads | Use `IDbContextFactory` in background services | Apply configurations via `IEntityTypeConfiguration<T>` | Use migrations for schema changes | Index frequently queried columns | ||
|
|
||
| **DON'T:** Use lazy loading in web apps (N+1 risk) | Call `SaveChanges` in a loop | Mix EF Core and raw SQL without reason | Use `Find()` for queries needing includes | Expose `DbContext` to controllers directly | Forget `CancellationToken` propagation | ||
|
|
||
| **Remember**: EF Core for most CRUD and schema management, Dapper for performance-critical reads. Always use parameterized queries, `AsNoTracking()` for reads, and projections to minimize data transfer. |
Contributor
There was a problem hiding this comment.
Add explicit “How It Works” and “Examples” sections.
This skill has When to Use (Line 11) but does not include explicit How It Works and Examples headings required by repo conventions. Please add those headings and group existing content/code blocks under them.
As per coding guidelines, “Skills should be formatted as Markdown with clear sections for When to Use, How It Works, and Examples”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/csharp-data-access/SKILL.md` around lines 20 - 410, The SKILL.md is
missing the required "How It Works" and "Examples" sections; add two new
Markdown headings "How It Works" (after the existing "When to Use" heading) and
"Examples" and reorganize content so explanatory text (concepts, best practices,
lifecycle explanations) goes under "How It Works" while concrete code blocks and
runnable snippets (DbContext Setup, DbContext Lifetime, Entity Configuration,
Query Optimization examples, Pagination, Migrations, Transactions, Dapper,
Repository Pattern, Auditing) are grouped under "Examples"; ensure headings are
top-level and preserve existing code blocks and headings like "DbContext Setup",
"Entity Configuration", "Query Optimization", "Migrations", "Transactions",
"Dapper", and "Repository Pattern" by moving them into the new "Examples" or
"How It Works" sections appropriately.
Comment on lines
+23
to
+25
| // Program.cs — register with connection pooling | ||
| builder.Services.AddDbContext<AppDbContext>(options => | ||
| options.UseNpgsql(builder.Configuration.GetConnectionString("Default"), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In ASP.NET Core / EF Core, does AddDbContext()enable DbContext pooling, or is pooling only enabled withAddDbContextPool()?
💡 Result:
In ASP.NET Core with EF Core, AddDbContext does NOT enable DbContext pooling. Pooling is only enabled by explicitly using AddDbContextPool. Official Microsoft documentation explicitly states: "To enable context pooling, simply replace AddDbContext with AddDbContextPool." This distinction has remained consistent across EF Core versions, including up to EF Core 9 (as of 2026 docs), with no changes making pooling automatic in AddDbContext. AddDbContext registers the DbContext as a scoped service, creating a new instance per DI scope (typically per HTTP request). AddDbContextPool registers a pool of reusable instances (default max 1024), reducing creation/disposal overhead for better performance in high-throughput scenarios. All sources, including performance guides, Stack Overflow (citing MS docs), and API references, confirm this separation. No evidence of pooling being enabled by default in AddDbContext in any version.
Citations:
- 1: https://aka.ms/efcore-docs-dbcontext-pooling
- 2: https://stackoverflow.com/questions/48443567/adddbcontext-or-adddbcontextpool
- 3: https://learn.microsoft.com/en-us/ef/core/performance/advanced-performance-topics
- 4: https://learn.microsoft.com/en-us/ef/core/dbcontext-configuration/
- 5: https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.entityframeworkservicecollectionextensions.adddbcontextpool?view=efcore-7.0
Remove "connection pooling" from comment or use AddDbContextPool.
The comment on line 23 claims "register with connection pooling", but AddDbContext does not enable pooling. Only AddDbContextPool activates pooling. Update the comment to reflect what the code actually does, or replace AddDbContext with AddDbContextPool if pooling is intended.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/csharp-data-access/SKILL.md` around lines 23 - 25, The comment says
"register with connection pooling" but the code uses AddDbContext, which does
not enable pooling; either update the comment to correctly state that
AppDbContext is registered without pooling, or switch the registration to
AddDbContextPool<AppDbContext>(...) to enable pooling (keep the same
options.UseNpgsql(...) configuration). Locate the registration call
(AddDbContext/AddDbContextPool) in Program.cs where AppDbContext is being
configured and apply the chosen change so the comment and implementation match.
Comment on lines
+20
to
+352
| ## Authentication & Authorization | ||
|
|
||
| ### ASP.NET Core Identity Setup | ||
|
|
||
| ```csharp | ||
| // Program.cs | ||
| builder.Services.AddIdentity<ApplicationUser, IdentityRole>(options => | ||
| { | ||
| options.Password.RequireDigit = true; | ||
| options.Password.RequiredLength = 12; | ||
| options.Password.RequireNonAlphanumeric = true; | ||
| options.Password.RequireUppercase = true; | ||
| options.Password.RequireLowercase = true; | ||
| options.Lockout.MaxFailedAccessAttempts = 5; | ||
| options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromMinutes(15); | ||
| options.User.RequireUniqueEmail = true; | ||
| }) | ||
| .AddEntityFrameworkStores<AppDbContext>() | ||
| .AddDefaultTokenProviders(); | ||
| ``` | ||
|
|
||
| ### JWT Bearer Authentication | ||
|
|
||
| ```csharp | ||
| builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) | ||
| .AddJwtBearer(options => | ||
| { | ||
| options.TokenValidationParameters = new TokenValidationParameters | ||
| { | ||
| ValidateIssuer = true, | ||
| ValidateAudience = true, | ||
| ValidateLifetime = true, | ||
| ValidateIssuerSigningKey = true, | ||
| ValidIssuer = builder.Configuration["Jwt:Issuer"], | ||
| ValidAudience = builder.Configuration["Jwt:Audience"], | ||
| IssuerSigningKey = new SymmetricSecurityKey( | ||
| Encoding.UTF8.GetBytes(builder.Configuration["Jwt:Key"] | ||
| ?? throw new InvalidOperationException("Jwt:Key configuration is required"))), | ||
| ClockSkew = TimeSpan.Zero | ||
| }; | ||
| }); | ||
|
|
||
| app.UseAuthentication(); | ||
| app.UseAuthorization(); | ||
| ``` | ||
|
|
||
| ### Policy-Based Authorization | ||
|
|
||
| ```csharp | ||
| builder.Services.AddAuthorizationBuilder() | ||
| .AddPolicy("AdminOnly", policy => policy.RequireRole("Admin")) | ||
| .AddPolicy("PremiumUser", policy => | ||
| policy.RequireAssertion(context => | ||
| context.User.HasClaim(c => c.Type == "Subscription" && c.Value == "Premium"))); | ||
|
|
||
| [Authorize(Policy = "AdminOnly")] | ||
| [HttpDelete("users/{id}")] | ||
| public IActionResult DeleteUser(int id) { /* ... */ } | ||
| ``` | ||
|
|
||
| ## Input Validation | ||
|
|
||
| ### Data Annotations | ||
|
|
||
| ```csharp | ||
| public record CreateUserRequest | ||
| { | ||
| [Required(ErrorMessage = "Name is required")] | ||
| [StringLength(100, MinimumLength = 2)] | ||
| public required string Name { get; init; } | ||
|
|
||
| [Required] | ||
| [EmailAddress(ErrorMessage = "Invalid email format")] | ||
| [StringLength(254)] | ||
| public required string Email { get; init; } | ||
|
|
||
| [Required] | ||
| [Range(18, 120, ErrorMessage = "Age must be between 18 and 120")] | ||
| public required int Age { get; init; } | ||
| } | ||
| ``` | ||
|
|
||
| ### FluentValidation Integration | ||
|
|
||
| ```csharp | ||
| public class CreateOrderValidator : AbstractValidator<CreateOrderRequest> | ||
| { | ||
| public CreateOrderValidator() | ||
| { | ||
| RuleFor(x => x.ProductId).GreaterThan(0); | ||
| RuleFor(x => x.Quantity).InclusiveBetween(1, 1000); | ||
| RuleFor(x => x.ShippingAddress).NotEmpty().MaximumLength(500); | ||
| RuleFor(x => x.Email).EmailAddress(EmailValidationMode.AspNetCoreCompatible); | ||
| } | ||
| } | ||
|
|
||
| // Program.cs | ||
| builder.Services.AddFluentValidationAutoValidation(); | ||
| builder.Services.AddValidatorsFromAssemblyContaining<CreateOrderValidator>(); | ||
| ``` | ||
|
|
||
| `[ApiController]` returns 400 automatically for invalid models — no manual `ModelState` check needed. | ||
|
|
||
| ## SQL Injection Prevention | ||
|
|
||
| ### EF Core LINQ (Automatically Parameterized) | ||
|
|
||
| ```csharp | ||
| // Good: LINQ queries are always parameterized | ||
| public async Task<List<User>> SearchUsers(string name, string status) | ||
| { | ||
| return await context.Users | ||
| .Where(u => u.Name.Contains(name) && u.Status == status) | ||
| .OrderBy(u => u.Name) | ||
| .Take(50) | ||
| .ToListAsync(); | ||
| } | ||
|
|
||
| // Good: FromSqlInterpolated — safely parameterized | ||
| public async Task<List<User>> FindByEmail(string email) | ||
| { | ||
| return await context.Users | ||
| .FromSqlInterpolated($"SELECT * FROM Users WHERE Email = {email}") | ||
| .ToListAsync(); | ||
| } | ||
|
|
||
| // Bad: FromSqlRaw with string interpolation (SQLi!) | ||
| return await context.Users | ||
| .FromSqlRaw($"SELECT * FROM Users WHERE Email = '{email}'") | ||
| .ToListAsync(); | ||
| ``` | ||
|
|
||
| ### Dapper with Parameters | ||
|
|
||
| ```csharp | ||
| // Good: Parameterized query | ||
| return await db.QuerySingleOrDefaultAsync<User>( | ||
| "SELECT * FROM Users WHERE Id = @Id", new { Id = id }); | ||
|
|
||
| // Bad: String concatenation (SQLi!) | ||
| return await db.QuerySingleOrDefaultAsync<User>( | ||
| "SELECT * FROM Users WHERE Id = " + id); | ||
| ``` | ||
|
|
||
| ## XSS Prevention | ||
|
|
||
| ```csharp | ||
| // Good: Razor @ automatically HTML-encodes output | ||
| <p>Welcome, @Model.UserName</p> | ||
|
|
||
| // Bad: @Html.Raw() with user data bypasses encoding (XSS!) | ||
| <div>@Html.Raw(Model.UserBio)</div> | ||
|
|
||
| // Good: Only use Html.Raw for trusted, sanitized content | ||
| <div>@Html.Raw(sanitizer.Sanitize(Model.UserBio))</div> | ||
|
|
||
| // Manual encoding for different contexts | ||
| using System.Text.Encodings.Web; | ||
| HtmlEncoder.Default.Encode(userInput); // HTML context | ||
| JavaScriptEncoder.Default.Encode(userInput); // JS context | ||
| UrlEncoder.Default.Encode(userInput); // URL context | ||
| ``` | ||
|
|
||
| ### Content-Security-Policy | ||
|
|
||
| ```csharp | ||
| app.Use(async (context, next) => | ||
| { | ||
| context.Response.Headers.Append( | ||
| "Content-Security-Policy", | ||
| "default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; frame-ancestors 'none'"); | ||
| await next(); | ||
| }); | ||
| ``` | ||
|
|
||
| ## CSRF Protection | ||
|
|
||
| ```csharp | ||
| // Razor form with anti-forgery token | ||
| <form method="post" asp-action="Transfer"> | ||
| @Html.AntiForgeryToken() | ||
| <input type="text" asp-for="Amount" /> | ||
| <button type="submit">Transfer</button> | ||
| </form> | ||
|
|
||
| [HttpPost] | ||
| [ValidateAntiForgeryToken] | ||
| public IActionResult Transfer(TransferRequest request) { /* ... */ } | ||
|
|
||
| // SameSite cookies | ||
| builder.Services.ConfigureApplicationCookie(options => | ||
| { | ||
| options.Cookie.HttpOnly = true; | ||
| options.Cookie.SecurePolicy = CookieSecurePolicy.Always; | ||
| options.Cookie.SameSite = SameSiteMode.Strict; | ||
| options.ExpireTimeSpan = TimeSpan.FromHours(1); | ||
| }); | ||
|
|
||
| // JWT-protected APIs don't need anti-forgery tokens when JWT is sent via Authorization header (not cookies) | ||
| [ApiController] | ||
| [Authorize(AuthenticationSchemes = JwtBearerDefaults.AuthenticationScheme)] | ||
| [IgnoreAntiforgeryToken] | ||
| public class OrdersApiController : ControllerBase { /* ... */ } | ||
| ``` | ||
|
|
||
| ## Secrets Management | ||
|
|
||
| ```bash | ||
| # Development: User Secrets (never stored in source code) | ||
| dotnet user-secrets init | ||
| dotnet user-secrets set "Database:Password" "my-dev-password" | ||
| dotnet user-secrets set "Jwt:Key" "my-dev-signing-key-at-least-32-chars" | ||
| ``` | ||
|
|
||
| ```csharp | ||
| // Production: Azure Key Vault | ||
| builder.Configuration.AddAzureKeyVault( | ||
| new Uri($"https://{builder.Configuration["KeyVault:Name"]}.vault.azure.net/"), | ||
| new DefaultAzureCredential()); | ||
|
|
||
| // IOptions pattern — bind and validate at startup | ||
| builder.Services.AddOptions<JwtSettings>() | ||
| .BindConfiguration("Jwt") | ||
| .ValidateDataAnnotations() | ||
| .ValidateOnStart(); | ||
| ``` | ||
|
|
||
| Never store secrets in `appsettings.json` — it's committed to source control. | ||
|
|
||
| ## HTTP Security Headers | ||
|
|
||
| ```csharp | ||
| // SecurityHeadersMiddleware.cs | ||
| public class SecurityHeadersMiddleware(RequestDelegate next) | ||
| { | ||
| public async Task InvokeAsync(HttpContext context) | ||
| { | ||
| var headers = context.Response.Headers; | ||
| headers.Append("X-Content-Type-Options", "nosniff"); | ||
| headers.Append("X-Frame-Options", "DENY"); | ||
| headers.Append("Referrer-Policy", "strict-origin-when-cross-origin"); | ||
| headers.Append("Permissions-Policy", "camera=(), microphone=(), geolocation=()"); | ||
| headers.Append("Content-Security-Policy", | ||
| "default-src 'self'; script-src 'self'; style-src 'self'; frame-ancestors 'none'"); | ||
| await next(context); | ||
| } | ||
| } | ||
|
|
||
| // HSTS (production only) | ||
| builder.Services.AddHsts(options => | ||
| { | ||
| options.MaxAge = TimeSpan.FromDays(365); | ||
| options.IncludeSubDomains = true; | ||
| options.Preload = true; | ||
| }); | ||
| app.UseHsts(); | ||
| app.UseHttpsRedirection(); | ||
| ``` | ||
|
|
||
| ## Rate Limiting | ||
|
|
||
| ```csharp | ||
| builder.Services.AddRateLimiter(options => | ||
| { | ||
| options.AddFixedWindowLimiter("fixed", o => | ||
| { | ||
| o.PermitLimit = 100; | ||
| o.Window = TimeSpan.FromMinutes(1); | ||
| }); | ||
| options.AddTokenBucketLimiter("token", o => | ||
| { | ||
| o.TokenLimit = 50; | ||
| o.ReplenishmentPeriod = TimeSpan.FromSeconds(10); | ||
| o.TokensPerPeriod = 10; | ||
| }); | ||
| options.RejectionStatusCode = StatusCodes.Status429TooManyRequests; | ||
| }); | ||
|
|
||
| app.UseRateLimiter(); | ||
|
|
||
| [EnableRateLimiting("fixed")] | ||
| [HttpPost("login")] | ||
| public IActionResult Login(LoginRequest request) { /* ... */ } | ||
| ``` | ||
|
|
||
| ## Security Tooling | ||
|
|
||
| ```xml | ||
| <!-- .csproj — enable security analyzers --> | ||
| <PropertyGroup> | ||
| <EnableNETAnalyzers>true</EnableNETAnalyzers> | ||
| <AnalysisLevel>latest-all</AnalysisLevel> | ||
| <TreatWarningsAsErrors>true</TreatWarningsAsErrors> | ||
| </PropertyGroup> | ||
| ``` | ||
|
|
||
| ```bash | ||
| # Check for known vulnerable packages | ||
| dotnet list package --vulnerable --include-transitive | ||
| ``` | ||
|
|
||
| ## OWASP Top 10 Mapping | ||
|
|
||
| | OWASP Category | C# / .NET Mitigation | | ||
| |---|---| | ||
| | A01: Broken Access Control | `[Authorize]` policies, resource-based authorization | | ||
| | A02: Cryptographic Failures | Data Protection API, Azure Key Vault | | ||
| | A03: Injection | EF Core LINQ, `FromSqlInterpolated`, Dapper parameters | | ||
| | A04: Insecure Design | Input validation with FluentValidation, secure defaults | | ||
| | A05: Security Misconfiguration | `UseHsts()`, security headers middleware | | ||
| | A06: Vulnerable Components | `dotnet list package --vulnerable`, NuGet audit | | ||
| | A07: Auth Failures | Identity with lockout, JWT with short expiry, MFA | | ||
| | A08: Software Integrity | NuGet package signing, CI artifact verification | | ||
| | A09: Logging Failures | Structured logging, audit trails, never log secrets | | ||
| | A10: SSRF | `HttpClient` with allowed-host validation | | ||
|
|
||
| ## Quick Security Checklist | ||
|
|
||
| | Check | What to Verify | | ||
| |---|---| | ||
| | Authentication | Identity or JWT with strong settings | | ||
| | Authorization | `[Authorize]` with policies on all protected endpoints | | ||
| | Input validation | Data Annotations or FluentValidation on all models | | ||
| | SQL queries | EF Core LINQ or parameterized queries only | | ||
| | HTML output | Razor auto-encoding, no `@Html.Raw()` with user data | | ||
| | CSRF | Anti-forgery tokens on forms, SameSite cookies | | ||
| | Secrets | User-secrets in dev, Key Vault in prod | | ||
| | HTTPS | `UseHttpsRedirection()` and `UseHsts()` | | ||
| | Headers | X-Content-Type-Options, X-Frame-Options, CSP | | ||
| | Rate limiting | `AddRateLimiter` on auth and sensitive endpoints | | ||
| | Dependencies | `dotnet list package --vulnerable` | | ||
|
|
||
| **Remember**: ASP.NET Core provides strong security defaults, but they must be configured correctly. Defense in depth — never rely on a single layer. |
Contributor
There was a problem hiding this comment.
Add required skill structure sections (“How It Works”, “Examples”).
The content is comprehensive, but the skill format is missing explicit How It Works and Examples headings.
As per coding guidelines, “Skills should be formatted as Markdown with clear sections for When to Use, How It Works, and Examples”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/csharp-security/SKILL.md` around lines 20 - 352, The skill markdown is
missing the required "How It Works" and "Examples" sections; update SKILL.md to
add a "How It Works" section explaining the core concepts and flows (e.g.,
Identity + JWT validation, policy-based auth, input validation, SQL/XSS/CSRF
mitigations) and an "Examples" section with concise usage snippets or scenarios
that reference existing examples (e.g., the Program.cs Identity/JWT blocks,
CreateUserRequest/FluentValidation snippets, SecurityHeadersMiddleware); ensure
the file follows the required skill format with "When to Use", "How It Works",
and "Examples" headings in Markdown and place them logically near the top-level
sections so reviewers can find them easily.
Comment on lines
+268
to
+276
| // HSTS (production only) | ||
| builder.Services.AddHsts(options => | ||
| { | ||
| options.MaxAge = TimeSpan.FromDays(365); | ||
| options.IncludeSubDomains = true; | ||
| options.Preload = true; | ||
| }); | ||
| app.UseHsts(); | ||
| app.UseHttpsRedirection(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In ASP.NET Core, should UseHsts()typically be wrapped withif (!app.Environment.IsDevelopment())?
💡 Result:
Yes, UseHsts should typically be wrapped with if (!app.Environment.IsDevelopment) in ASP.NET Core applications.
Citations:
🏁 Script executed:
# Find and examine the skills file
find . -path "*skills/csharp-security*" -type f | head -20Repository: affaan-m/everything-claude-code
Length of output: 108
🏁 Script executed:
# Check the file size and read the specific lines
wc -l ./skills/csharp-security/SKILL.md
echo "---"
sed -n '260,280p' ./skills/csharp-security/SKILL.mdRepository: affaan-m/everything-claude-code
Length of output: 714
🏁 Script executed:
# Check file structure and naming compliance
echo "=== File naming check ==="
ls -la ./skills/csharp-security/ | grep -i skill
echo "=== Markdown structure (first 50 lines) ==="
head -50 ./skills/csharp-security/SKILL.md
echo "=== Look for section headers ==="
grep -n "^##" ./skills/csharp-security/SKILL.md | head -20Repository: affaan-m/everything-claude-code
Length of output: 2482
🏁 Script executed:
# Check if there are explicit "How It Works" and "Examples" sections in the file
grep -n "^## How It Works\|^## Examples\|^### Examples" ./skills/csharp-security/SKILL.mdRepository: affaan-m/everything-claude-code
Length of output: 57
Guard HSTS middleware with environment check.
The HSTS configuration is marked as "production only," but app.UseHsts() is called unconditionally. Per ASP.NET Core security best practices, HSTS must be applied only in non-development environments to avoid issues with local development workflows.
Suggested fix
// HSTS (production only)
builder.Services.AddHsts(options =>
{
options.MaxAge = TimeSpan.FromDays(365);
options.IncludeSubDomains = true;
options.Preload = true;
});
-app.UseHsts();
+if (!app.Environment.IsDevelopment())
+{
+ app.UseHsts();
+}
app.UseHttpsRedirection();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // HSTS (production only) | |
| builder.Services.AddHsts(options => | |
| { | |
| options.MaxAge = TimeSpan.FromDays(365); | |
| options.IncludeSubDomains = true; | |
| options.Preload = true; | |
| }); | |
| app.UseHsts(); | |
| app.UseHttpsRedirection(); | |
| // HSTS (production only) | |
| builder.Services.AddHsts(options => | |
| { | |
| options.MaxAge = TimeSpan.FromDays(365); | |
| options.IncludeSubDomains = true; | |
| options.Preload = true; | |
| }); | |
| if (!app.Environment.IsDevelopment()) | |
| { | |
| app.UseHsts(); | |
| } | |
| app.UseHttpsRedirection(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/csharp-security/SKILL.md` around lines 268 - 276, HSTS is configured
but app.UseHsts() is called unconditionally; wrap the HSTS setup and call so it
only runs in non-development environments (use the application environment
check, e.g., app.Environment.IsDevelopment()/IWebHostEnvironment.IsDevelopment)
so that builder.Services.AddHsts(...) and app.UseHsts() are executed only when
not in development while leaving app.UseHttpsRedirection() as appropriate.
Comment on lines
+19
to
+411
| ## TDD Workflow | ||
|
|
||
| Always follow the RED-GREEN-REFACTOR cycle. | ||
|
|
||
| ```csharp | ||
| // Step 1: RED — Write a failing test | ||
| using Xunit; | ||
| using FluentAssertions; | ||
|
|
||
| namespace MyApp.Tests.Unit; | ||
|
|
||
| public class CalculatorTests | ||
| { | ||
| [Fact] | ||
| public void Add_TwoPositiveNumbers_ReturnsSum() | ||
| { | ||
| var calculator = new Calculator(); | ||
| calculator.Add(2, 3).Should().Be(5); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(-1, 1, 0)] | ||
| [InlineData(-5, -3, -8)] | ||
| [InlineData(0, 0, 0)] | ||
| public void Add_VariousInputs_ReturnsExpectedSum(int a, int b, int expected) | ||
| { | ||
| new Calculator().Add(a, b).Should().Be(expected); | ||
| } | ||
| } | ||
|
|
||
| // Step 2: GREEN — Write minimal implementation | ||
| public class Calculator | ||
| { | ||
| public int Add(int a, int b) => a + b; | ||
| } | ||
|
|
||
| // Step 3: REFACTOR — Improve while tests stay green | ||
| ``` | ||
|
|
||
| ```bash | ||
| dotnet test # Run all tests | ||
| dotnet test --filter "ClassName" # Run specific class | ||
| dotnet watch test # Re-run on file changes | ||
| ``` | ||
|
|
||
| ## xUnit Fundamentals | ||
|
|
||
| ### Facts, Theories, and MemberData | ||
|
|
||
| ```csharp | ||
| public class UserServiceTests | ||
| { | ||
| // [Fact] — single test case | ||
| [Fact] | ||
| public void GetDisplayName_WithFirstAndLastName_ReturnsCombined() | ||
| { | ||
| var user = new User("Alice", "Smith"); | ||
| user.GetDisplayName().Should().Be("Alice Smith"); | ||
| } | ||
|
|
||
| // [Theory] with [InlineData] — parameterized tests | ||
| [Theory] | ||
| [InlineData("", false)] | ||
| [InlineData("abcd1!", true)] | ||
| public void IsValidPassword_VariousInputs_ReturnsExpected(string password, bool expected) | ||
| { | ||
| new PasswordValidator().IsValid(password).Should().Be(expected); | ||
| } | ||
|
|
||
| // [MemberData] — data from a static property | ||
| public static IEnumerable<object[]> DivisionData => | ||
| [ | ||
| [10.0, 2.0, 5.0], | ||
| [9.0, 3.0, 3.0], | ||
| ]; | ||
|
|
||
| [Theory] | ||
| [MemberData(nameof(DivisionData))] | ||
| public void Divide_ValidInputs_ReturnsQuotient(double a, double b, double expected) | ||
| { | ||
| new MathService().Divide(a, b).Should().Be(expected); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Test Naming Convention | ||
|
|
||
| ``` | ||
| {MethodUnderTest}_{Scenario}_{ExpectedBehavior} | ||
| ``` | ||
|
|
||
| - Class: `{ClassUnderTest}Tests` (e.g., `OrderServiceTests`) | ||
| - Method: `PlaceOrder_EmptyCart_ThrowsInvalidOperationException` | ||
|
|
||
| ### IAsyncLifetime for Async Setup/Teardown | ||
|
|
||
| ```csharp | ||
| public class DatabaseTests : IAsyncLifetime | ||
| { | ||
| private DbConnection _connection = null!; | ||
|
|
||
| public async Task InitializeAsync() | ||
| { | ||
| _connection = new SqliteConnection("Data Source=:memory:"); | ||
| await _connection.OpenAsync(); | ||
| await SeedTestData(_connection); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task QueryUsers_ReturnsSeededData() | ||
| { | ||
| var users = await QueryUsers(_connection); | ||
| users.Should().NotBeEmpty(); | ||
| } | ||
|
|
||
| public async Task DisposeAsync() => await _connection.DisposeAsync(); | ||
|
|
||
| private static Task SeedTestData(DbConnection conn) => Task.CompletedTask; // Insert test data here | ||
| private static Task<List<string>> QueryUsers(DbConnection conn) => Task.FromResult(new List<string> { "Alice" }); | ||
| } | ||
| ``` | ||
|
|
||
| ## FluentAssertions | ||
|
|
||
| ```csharp | ||
| // Value assertions | ||
| result.Should().Be(42); | ||
| result.Should().BeInRange(1, 100); | ||
|
|
||
| // String assertions | ||
| name.Should().StartWith("Alice").And.Contain("Smith"); | ||
|
|
||
| // Collection assertions | ||
| items.Should().HaveCount(3).And.Contain("banana").And.BeInAscendingOrder(); | ||
|
|
||
| // Object graph comparison | ||
| actual.Should().BeEquivalentTo(expected, o => o.Excluding(u => u.CreatedAt)); | ||
|
|
||
| // Exception assertions | ||
| var act = () => service.PlaceOrder(null!); | ||
| act.Should().Throw<ArgumentNullException>().WithMessage("*order*"); | ||
|
|
||
| // Async exception | ||
| Func<Task> asyncAct = () => service.ProcessAsync(null!); | ||
| await asyncAct.Should().ThrowAsync<InvalidOperationException>(); | ||
| ``` | ||
|
|
||
| ## NSubstitute | ||
|
|
||
| ```csharp | ||
| public class OrderServiceTests | ||
| { | ||
| private readonly IOrderRepository _repository = Substitute.For<IOrderRepository>(); | ||
| private readonly INotificationService _notifications = Substitute.For<INotificationService>(); | ||
| private readonly OrderService _sut; | ||
|
|
||
| public OrderServiceTests() | ||
| { | ||
| _sut = new OrderService(_repository, _notifications); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task PlaceOrder_ValidOrder_SavesAndNotifies() | ||
| { | ||
| var order = new Order("ORD-001", "Widget", 3); | ||
| _repository.SaveAsync(Arg.Any<Order>()).Returns(Task.FromResult(true)); | ||
|
|
||
| await _sut.PlaceOrderAsync(order); | ||
|
|
||
| await _repository.Received(1).SaveAsync(Arg.Is<Order>(o => o.Id == "ORD-001")); | ||
| await _notifications.Received(1).SendAsync(Arg.Is<string>(msg => msg.Contains("ORD-001"))); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task PlaceOrder_SaveFails_DoesNotNotify() | ||
| { | ||
| _repository.SaveAsync(Arg.Any<Order>()).Returns(Task.FromResult(false)); | ||
|
|
||
| await _sut.PlaceOrderAsync(new Order("ORD-002", "Gadget", 1)); | ||
|
|
||
| await _notifications.DidNotReceive().SendAsync(Arg.Any<string>()); | ||
| } | ||
| } | ||
|
|
||
| // Good: Substitute interfaces, not concrete classes | ||
| // var repo = Substitute.For<IOrderRepository>(); | ||
| ``` | ||
|
|
||
| ## Integration Testing | ||
|
|
||
| ### WebApplicationFactory | ||
|
|
||
| ```csharp | ||
| public class CustomWebApplicationFactory : WebApplicationFactory<Program> | ||
| { | ||
| protected override void ConfigureWebHost(IWebHostBuilder builder) | ||
| { | ||
| builder.ConfigureServices(services => | ||
| { | ||
| var descriptor = services.SingleOrDefault( | ||
| d => d.ServiceType == typeof(DbContextOptions<AppDbContext>)); | ||
| if (descriptor is not null) services.Remove(descriptor); | ||
|
|
||
| services.AddDbContext<AppDbContext>(options => | ||
| options.UseInMemoryDatabase($"TestDb_{Guid.NewGuid()}")); | ||
| services.AddSingleton<IEmailService, FakeEmailService>(); | ||
| }); | ||
| builder.UseEnvironment("Testing"); | ||
| } | ||
| } | ||
|
|
||
| public class OrdersApiTests : IClassFixture<CustomWebApplicationFactory> | ||
| { | ||
| private readonly HttpClient _client; | ||
|
|
||
| public OrdersApiTests(CustomWebApplicationFactory factory) | ||
| { | ||
| _client = factory.CreateClient(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task CreateOrder_ValidPayload_Returns201() | ||
| { | ||
| var response = await _client.PostAsJsonAsync("/api/orders", | ||
| new { ProductName = "Widget", Quantity = 3 }); | ||
|
|
||
| response.StatusCode.Should().Be(HttpStatusCode.Created); | ||
| var order = await response.Content.ReadFromJsonAsync<OrderResponse>(); | ||
| order!.ProductName.Should().Be("Widget"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task GetOrder_NonExistent_Returns404() | ||
| { | ||
| var response = await _client.GetAsync("/api/orders/99999"); | ||
| response.StatusCode.Should().Be(HttpStatusCode.NotFound); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### TestContainers for Database Testing | ||
|
|
||
| ```csharp | ||
| public class PostgresTests : IAsyncLifetime | ||
| { | ||
| private readonly PostgreSqlContainer _postgres = new PostgreSqlBuilder() | ||
| .WithImage("postgres:16-alpine") | ||
| .WithDatabase("testdb") | ||
| .WithUsername("test") | ||
| .WithPassword("test") | ||
| .Build(); | ||
|
|
||
| public async Task InitializeAsync() => await _postgres.StartAsync(); | ||
|
|
||
| [Fact] | ||
| public async Task UserRepository_Save_PersistsToDatabase() | ||
| { | ||
| var repo = new UserRepository(_postgres.GetConnectionString()); | ||
| await repo.SaveAsync(new User("Alice", "alice@test.com")); | ||
|
|
||
| var user = await repo.FindByEmailAsync("alice@test.com"); | ||
| user.Should().NotBeNull(); | ||
| user!.Name.Should().Be("Alice"); | ||
| } | ||
|
|
||
| public async Task DisposeAsync() => await _postgres.DisposeAsync(); | ||
| } | ||
| ``` | ||
|
|
||
| ## Test Organization | ||
|
|
||
| ### Arrange-Act-Assert Pattern | ||
|
|
||
| ```csharp | ||
| [Fact] | ||
| public void CalculateDiscount_GoldMember_Returns20Percent() | ||
| { | ||
| // Arrange | ||
| var pricing = new PricingService(); | ||
| var customer = new Customer("Alice", MembershipTier.Gold); | ||
| var order = new Order(totalAmount: 100.00m); | ||
|
|
||
| // Act | ||
| var discount = pricing.CalculateDiscount(customer, order); | ||
|
|
||
| // Assert | ||
| discount.Should().Be(20.00m); | ||
| } | ||
| ``` | ||
|
|
||
| ### Test Project Structure | ||
|
|
||
| ```text | ||
| tests/ | ||
| ├── Unit/ | ||
| │ ├── Services/ | ||
| │ └── Validators/ | ||
| ├── Integration/ | ||
| │ ├── Api/ | ||
| │ └── CustomWebApplicationFactory.cs | ||
| ├── E2E/ | ||
| └── MyApp.Tests.csproj | ||
| ``` | ||
|
|
||
| ### Shared Fixtures | ||
|
|
||
| ```csharp | ||
| // IClassFixture — shared across tests in one class | ||
| public class UserRepoTests : IClassFixture<DatabaseFixture> | ||
| { | ||
| public UserRepoTests(DatabaseFixture db) { /* use db.ConnectionString */ } | ||
| } | ||
|
|
||
| // ICollectionFixture — shared across multiple test classes | ||
| [CollectionDefinition("Database")] | ||
| public class DatabaseCollection : ICollectionFixture<DatabaseFixture> { } | ||
|
|
||
| [Collection("Database")] | ||
| public class OrderRepoTests { /* ... */ } | ||
| ``` | ||
|
|
||
| ## Coverage with Coverlet | ||
|
|
||
| ```bash | ||
| # Collect coverage | ||
| dotnet test --collect:"XPlat Code Coverage" | ||
|
|
||
| # Generate HTML report | ||
| dotnet tool install -g dotnet-reportgenerator-globaltool | ||
| reportgenerator -reports:"**/coverage.cobertura.xml" -targetdir:coveragereport -reporttypes:Html | ||
|
|
||
| # Filter by trait | ||
| dotnet test --filter "Category=Unit" | ||
| dotnet test --filter "Category!=Integration" | ||
| ``` | ||
|
|
||
| ## Best Practices | ||
|
|
||
| **DO:** Follow TDD (red-green-refactor) | Use FluentAssertions for readability | Use Arrange-Act-Assert | Mock interfaces, not classes | Use `IAsyncLifetime` for async setup | Name tests `Method_Scenario_Expected` | Aim for 80%+ coverage | Keep tests fast | ||
|
|
||
| **DON'T:** Test implementation details | Share mutable state between tests | Use static mutable state | Over-mock (mock boundaries only) | Skip `await` in async tests | Test framework code | Write brittle tests relying on exact strings | Skip `Dispose` | ||
|
|
||
| ## Quick Reference | ||
|
|
||
| | Task | Command / Pattern | | ||
| |---|---| | ||
| | Run all tests | `dotnet test` | | ||
| | Filter by class | `dotnet test --filter "FullyQualifiedName~OrderServiceTests"` | | ||
| | Watch mode | `dotnet watch test` | | ||
| | Coverage | `dotnet test --collect:"XPlat Code Coverage"` | | ||
| | Assert equality | `result.Should().Be(expected)` | | ||
| | Assert collection | `items.Should().HaveCount(3).And.Contain("x")` | | ||
| | Assert exception | `act.Should().Throw<ArgumentException>()` | | ||
| | Create substitute | `Substitute.For<IService>()` | | ||
| | Verify call | `service.Received(1).Method(Arg.Any<string>())` | | ||
| | Verify no call | `service.DidNotReceive().Method(Arg.Any<string>())` | | ||
|
|
||
| ## Common Pitfalls | ||
|
|
||
| ```csharp | ||
| // Bad: Missing await — test passes without running async code | ||
| [Fact] | ||
| public void GetUser_Returns_User() | ||
| { | ||
| var result = _service.GetUserAsync(1); // Returns Task, not User | ||
| } | ||
|
|
||
| // Good: Properly await | ||
| [Fact] | ||
| public async Task GetUser_ExistingId_ReturnsUser() | ||
| { | ||
| var result = await _service.GetUserAsync(1); | ||
| result.Should().NotBeNull(); | ||
| } | ||
|
|
||
| // Bad: Static shared state — order-dependent failures | ||
| public class BadTests | ||
| { | ||
| private static List<string> _items = []; | ||
| [Fact] public void Test1() { _items.Add("a"); _items.Should().HaveCount(1); } | ||
| [Fact] public void Test2() { _items.Add("b"); _items.Should().HaveCount(1); } // Fails! | ||
| } | ||
|
|
||
| // Good: Fresh state per test via constructor | ||
| public class GoodTests | ||
| { | ||
| private readonly List<string> _items = []; | ||
| [Fact] public void Test1() { _items.Add("a"); _items.Should().HaveCount(1); } | ||
| [Fact] public void Test2() { _items.Add("b"); _items.Should().HaveCount(1); } // Passes | ||
| } | ||
| ``` | ||
|
|
||
| **Remember**: Tests are your safety net. Keep them fast, focused, and independent. |
Contributor
There was a problem hiding this comment.
Restructure with explicit “How It Works” and “Examples” headers.
The document is detailed, but it should explicitly include How It Works and Examples section headings (alongside When to Use) to match skill format requirements.
As per coding guidelines, “Skills should be formatted as Markdown with clear sections for When to Use, How It Works, and Examples”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/csharp-testing/SKILL.md` around lines 19 - 411, The SKILL.md currently
has detailed content (sections like "TDD Workflow", "xUnit Fundamentals",
"FluentAssertions", "Integration Testing") but lacks explicit "How It Works" and
"Examples" headings required by our skill format; update the document by adding
a "How It Works" section that summarizes the workflow and testing principles
(extract from TDD Workflow, xUnit Fundamentals, IAsyncLifetime,
Arrange-Act-Assert, Best Practices) and an "Examples" section that collects
runnable examples (the RED-GREEN-REFACTOR snippet, xUnit Facts/Theories
examples, WebApplicationFactory and TestContainers snippets) while keeping or
adding a "When to Use" header near the top; ensure headings are clear Markdown
headers and move or duplicate relevant code blocks under the new sections so the
file follows the "When to Use / How It Works / Examples" structure.
affaan-m
force-pushed
the
community/pr-652-csharp-support
branch
from
b19912a to
6ddb864
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rules/csharp/coding-style.md`:
- Line 10: Update the sentence "This file extends
[common/coding-style.md](../common/coding-style.md) with C# specific content."
by hyphenating the compound adjective so it reads "C#-specific" (i.e., change
"C# specific" to "C#-specific") to follow correct compound-adjective styling.
In `@rules/csharp/hooks.md`:
- Line 10: Update the intro sentence "This file extends
[common/hooks.md](../common/hooks.md) with C# specific content." by replacing
the phrase "C# specific content" with the hyphenated compound adjective
"C#-specific content" so the line reads "This file extends
[common/hooks.md](../common/hooks.md) with C#-specific content."
In `@rules/csharp/security.md`:
- Line 10: Update the intro sentence in rules/csharp/security.md that currently
reads "This file extends [common/security.md] with C# specific content." to use
the hyphenated form "C#-specific" for consistency and readability; locate the
introductory line containing that phrase and replace "C# specific" with
"C#-specific".
In `@rules/csharp/testing.md`:
- Line 10: Update the sentence string "This file extends
[common/testing.md](../common/testing.md) with C# specific content." to
hyphenate "C#-specific" so it reads "This file extends
[common/testing.md](../common/testing.md) with C#-specific content." — edit the
exact sentence in rules/csharp/testing.md (look for the line starting with "This
file extends") to replace "C# specific" with "C#-specific".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 696a59ff-30af-4409-997c-4a284222c0b9
📒 Files selected for processing (12)
AGENTS.mdREADME.mdrules/README.mdrules/csharp/coding-style.mdrules/csharp/hooks.mdrules/csharp/patterns.mdrules/csharp/security.mdrules/csharp/testing.mdskills/csharp-data-access/SKILL.mdskills/csharp-patterns/SKILL.mdskills/csharp-security/SKILL.mdskills/csharp-testing/SKILL.md
✅ Files skipped from review due to trivial changes (6)
- AGENTS.md
- README.md
- skills/csharp-testing/SKILL.md
- skills/csharp-patterns/SKILL.md
- skills/csharp-data-access/SKILL.md
- skills/csharp-security/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- rules/README.md
| --- | ||
| # C# Coding Style | ||
|
|
||
| > This file extends [common/coding-style.md](../common/coding-style.md) with C# specific content. |
Contributor
There was a problem hiding this comment.
Minor docs wording fix: “C#-specific”.
Please hyphenate the compound adjective in this sentence.
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...e.md](../common/coding-style.md) with C# specific content. ## Standards - Targe...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/csharp/coding-style.md` at line 10, Update the sentence "This file
extends [common/coding-style.md](../common/coding-style.md) with C# specific
content." by hyphenating the compound adjective so it reads "C#-specific" (i.e.,
change "C# specific" to "C#-specific") to follow correct compound-adjective
styling.
| --- | ||
| # C# Hooks | ||
|
|
||
| > This file extends [common/hooks.md](../common/hooks.md) with C# specific content. |
Contributor
There was a problem hiding this comment.
Use hyphenated compound adjective in intro line.
Please change “C# specific content” to “C#-specific content” for consistency and readability.
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...on/hooks.md](../common/hooks.md) with C# specific content. ## PostToolUse Hooks ...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/csharp/hooks.md` at line 10, Update the intro sentence "This file
extends [common/hooks.md](../common/hooks.md) with C# specific content." by
replacing the phrase "C# specific content" with the hyphenated compound
adjective "C#-specific content" so the line reads "This file extends
[common/hooks.md](../common/hooks.md) with C#-specific content."
| --- | ||
| # C# Security | ||
|
|
||
| > This file extends [common/security.md](../common/security.md) with C# specific content. |
Contributor
There was a problem hiding this comment.
Use “C#-specific” in the intro sentence.
Small readability/consistency fix: hyphenate “C# specific”.
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...urity.md](../common/security.md) with C# specific content. ## Secret Management ...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/csharp/security.md` at line 10, Update the intro sentence in
rules/csharp/security.md that currently reads "This file extends
[common/security.md] with C# specific content." to use the hyphenated form
"C#-specific" for consistency and readability; locate the introductory line
containing that phrase and replace "C# specific" with "C#-specific".
| --- | ||
| # C# Testing | ||
|
|
||
| > This file extends [common/testing.md](../common/testing.md) with C# specific content. |
Contributor
There was a problem hiding this comment.
Hyphenate “C#-specific” in the extension sentence.
This is a small docs polish: “C# specific content” → “C#-specific content”.
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...esting.md](../common/testing.md) with C# specific content. ## Framework Use **x...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/csharp/testing.md` at line 10, Update the sentence string "This file
extends [common/testing.md](../common/testing.md) with C# specific content." to
hyphenate "C#-specific" so it reads "This file extends
[common/testing.md](../common/testing.md) with C#-specific content." — edit the
exact sentence in rules/csharp/testing.md (look for the line starting with "This
file extends") to replace "C# specific" with "C#-specific".
6ddb864 to
f99825f
Compare
- Replace HttpClient anti-pattern with IHttpClientFactory in async example - Fix misleading "no heap allocation" claim for record structs - Replace null-forgiving operator with fail-fast pattern for Jwt:Key config - Remove unsafe-inline from CSP example to prevent XSS - Clarify JWT CSRF guidance applies only when using Authorization header - Use unique in-memory database names for test isolation - Fix internally inconsistent test stub (empty list vs NotBeEmpty assert) - Rename "When to Activate" to "When to Use" per skill format convention
affaan-m
force-pushed
the
community/pr-652-csharp-support
branch
from
f99825f to
8785e9f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 207: The README sentence "✨ **That's it!** You now have access to 27
agents, 113 skills, and 57 commands." is stale; update those counts to the PR's
final catalog numbers: "27 agents, 117 skills, and 58 commands." Edit that exact
line in README.md and also search the repository for any other occurrences of
the old totals and update them to keep docs consistent with the catalog state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88397d81-79a6-4891-959d-56580e13b371
📒 Files selected for processing (12)
AGENTS.mdREADME.mdrules/README.mdrules/csharp/coding-style.mdrules/csharp/hooks.mdrules/csharp/patterns.mdrules/csharp/security.mdrules/csharp/testing.mdskills/csharp-data-access/SKILL.mdskills/csharp-patterns/SKILL.mdskills/csharp-security/SKILL.mdskills/csharp-testing/SKILL.md
✅ Files skipped from review due to trivial changes (5)
- rules/README.md
- skills/csharp-security/SKILL.md
- skills/csharp-testing/SKILL.md
- skills/csharp-patterns/SKILL.md
- skills/csharp-data-access/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
| ``` | ||
|
|
||
| ✨ **That's it!** You now have access to 27 agents, 113 skills, and 58 commands. | ||
| ✨ **That's it!** You now have access to 27 agents, 113 skills, and 57 commands. |
Contributor
There was a problem hiding this comment.
Catalog totals appear stale after C# expansion.
Line 207 reports 27 agents, 113 skills, and 57 commands, but this PR’s stated target is 27 agents, 117 skills, and 58 commands. Please sync the README totals to the final catalog state to avoid installation/expectation drift.
📌 Suggested doc fix
-✨ **That's it!** You now have access to 27 agents, 113 skills, and 57 commands.
+✨ **That's it!** You now have access to 27 agents, 117 skills, and 58 commands.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ✨ **That's it!** You now have access to 27 agents, 113 skills, and 57 commands. | |
| ✨ **That's it!** You now have access to 27 agents, 117 skills, and 58 commands. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 207, The README sentence "✨ **That's it!** You now have
access to 27 agents, 113 skills, and 57 commands." is stale; update those counts
to the PR's final catalog numbers: "27 agents, 117 skills, and 58 commands."
Edit that exact line in README.md and also search the repository for any other
occurrences of the old totals and update them to keep docs consistent with the
catalog state.
Owner
Author
|
Will re-apply on fresh branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Original PR
Closes #652
Test plan
Summary by cubic
Adds C#/.NET 8+ language support with five new rules and four skills, plus updated docs and installers. Catalog totals now show 27 agents, 113 skills, 57 commands, and 39 rules; all tests pass.
New Features
rules/csharp/:coding-style,patterns,security,testing,hooks.csharp-patterns,csharp-security,csharp-testing,csharp-data-access(EF Core + Dapper).install.sh/install.ps1support,rules/README.mdsteps, directory listings, README tables/counters, and Cursor instructions.Bug Fixes
HttpClientanti-pattern withIHttpClientFactory.unsafe-inline); fail-fast for missingJwt:Key.Written for commit 8785e9f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Chores