mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
eval-exec-opus48
589 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dan Moseley
|
fd1dc612d0 |
Clarify that when-to-use guidance belongs in description frontmatter (#165)
* Clarify that when-to-use guidance belongs in description frontmatter The agent runtime reads the description field to decide whether to load a skill. Moving when-to-use/not-use guidance into the description avoids loading the full SKILL.md unnecessarily, reducing token load. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CONTRIBUTING.md frontmatter example and tip consistency - Update description placeholder to include 'when not to use' guidance - Remove brittle line-number anchors from SKILL.md link - Change 'token load' to 'token usage' for clarity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Aaron R Robinson
|
dc85d453c4 |
Add missing CODEOWNERS entries for new skills (#161)
* Add missing CODEOWNERS entries for new skills Add folder-level CODEOWNERS entries for: - android-tombstone-symbolication (@jeffschwMSFT) - clr-activation-debugging (@marklio) - microbenchmarking (@caaavik-msft) Fixes #159 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Apply suggestion from @AaronRobinsonMSFT * Apply suggestion from @AaronRobinsonMSFT * Apply suggestion from @AaronRobinsonMSFT * Apply suggestion from @AaronRobinsonMSFT --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Jan Krivanek
|
7b2708f8c7 |
[Infra] Add devops agentic workflow (#141)
* Add initial plan * add token strategy * Add devops agentic workflow PoC * Fix PR Comments * Replace hand-written .yml wrappers with gh aw compile - Move workflow .md files from eng/agentic-workflows/ to .github/workflows/ - Delete hand-written .yml wrappers (devops-health-check.yml, devops-health-investigate.yml) - Generate .lock.yml files via gh aw compile - Move compiled knowledge files to .github/aw/shared/ (required by gh-aw import security) - Fix permissions: issues: write -> issues: read (strict mode uses safe-outputs) - Update README with new file structure and setup instructions Addresses PR #141 Comment 5 (ViktorHofer) * Use fuzzy daily schedule, update README for gh-aw standard structure * Recompile lock.yml with latest changes * Add skills health * Remove redundant file |
||
|
Jared Parsons
|
4793aa27e9 |
Replace binlog mcp with replay to textlog (#162)
Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com> |
||
|
Jared Parsons
|
992dd7f3c9 |
GHA to validate ownership (#158)
* GHA to validate ownership Part of our contribution policy is that every skill / plugin has an associated owner with it. This GHA is designed to block PRs that produce content that violates this. * Update .github/workflows/codeowners-folder-validation.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Jared Parsons <jared@paranoidcoding.org> * better output --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Jeff Schwartz
|
ac12ae8a6a |
Move android-tombstone-symbolication skill and tests from src/dotnet to plugins/dotnet and tests/dotnet (#157)
Move the android-tombstone-symbolication skill from src/dotnet/skills/ to plugins/dotnet/skills/ and its tests from src/dotnet/tests/ to tests/dotnet/ to align with the standard plugin directory structure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Steve Pfister
|
59ac9ec41f |
Add android-tombstone-symbolication skill (#130)
* Add android-tombstone-symbolication skill Adds a skill for symbolicating .NET runtime frames in Android tombstone files. Parses native backtrace frames, downloads debug symbols from Microsoft's public symbol server using ELF BuildIds, and runs llvm-symbolizer to resolve function names and source locations. Includes: - SKILL.md with 5-step workflow covering Mono, CoreCLR, and NativeAOT - PowerShell automation script with -ParseOnly and -CrashingThreadOnly modes - 11 eval scenarios with 8 fixture files (Mono, CoreCLR, NativeAOT, multi-thread, multi-BuildId, no-BuildId, no-.NET negative, iOS negative) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Check ANDROID_NDK_ROOT and ANDROID_HOME for llvm-symbolizer Script now probes NDK paths via ANDROID_NDK_ROOT and ANDROID_HOME environment variables before failing. SKILL.md updated to guide the agent to check NDK paths first. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add .NET runtime version identification to tombstone symbolication - Add Find-RuntimeVersion: searches local SDK packs and NuGet cache for matching BuildId to identify exact .NET version and commit hash - Add Find-RuntimeVersionOnline: NuGet.org API fallback when runtime pack is not installed locally - Add -SkipVersionLookup switch to opt out of version identification - Report version and commit URL in symbolicated output footer - Add INVOKES routing signal and Stop Signals section to SKILL.md - Inline critical anti-patterns near relevant workflow steps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Compress SKILL.md and fix script eval setup - Compress SKILL.md from ~4,085 to ~2,048 tokens (50% reduction) while preserving all meaning. Removes redundancy, tightens prose, shortens examples, and deduplicates pitfalls. - Fix eval.yaml script scenarios to copy Symbolicate-Tombstone.ps1 into the agent working directory via the files/source setup option. Previously the script was missing from the work dir, causing flaky pass/fail depending on whether the agent found it by searching. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Dan Moseley
|
63c6ee1eec |
Nullable reference migration skill (#39)
* Add nullable-reference-migration skill Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: annotate based on design intent, not warnings Annotations should reflect the intended nullability contract of each member. Deciding intent first and then fixing warnings avoids the common mistake of over-annotating with ? or scattering ! to silence the compiler. Learned from dotnet/runtime nullable annotation guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: ?. (null-conditional) changes behavior unlike ? and ! During nullable migration it is tempting to change obj.Method() to obj?.Method() to silence a warning. Unlike ? on a type or ! on an expression, ?. changes the generated IL and silently skips the call when the value is null, which can introduce subtle regressions. Learned from dotnet/runtime PR #57418, #65262, #67198 review comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: adding ? to value types changes the actual type For reference types, ? is a metadata-only annotation. For value types (enums, structs, int, etc.), ? wraps the type in Nullable<T>, which changes the method signature and binary layout. This was caught as a breaking change in dotnet/runtime PR #65262 when LogLevel was accidentally made nullable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: Debug.Assert as compiler hint and reducing ! usage Debug.Assert(x != null) informs the nullable flow analyzer just like an if-check, eliminating the need for ! operators in the subsequent scope. Also adds guidance that if every call site of an internal method needs !, the parameter should probably be nullable instead. Learned from dotnet/runtime PR #67198 and PR #67198 review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Enhance CS8618 guidance with [MemberNotNull] for helper methods When a helper method called from the constructor initializes fields, [MemberNotNull(nameof(field))] tells the compiler those fields are guaranteed non-null after the call, eliminating CS8618 warnings without resorting to ! or making fields nullable. Learned from dotnet/runtime PR #67198 where this pattern eliminated many suppression operators. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for struct default fields and post-Dispose state Struct reference-type fields are null in default(T). Annotate based on whether default is valid usage: nullable if yes, non-nullable if not. Also adds guidance that post-Dispose nullability should not weaken annotations, since post-Dispose use is a contract violation. Learned from dotnet/runtime PR #528 (struct fields), nullability.md (InterfaceMapping vs CancellationToken), and PR #65262 (Dispose). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: prioritize public API annotations over internal code Public and protected APIs define the contract consumers depend on and should be annotated first and most carefully. Internal code can tolerate ! more liberally since it does not affect callers. Learned from dotnet/runtime PR #67198 review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: do not remove ArgumentNullException checks Non-nullable parameters still need runtime null validation because callers may not have NRT enabled (other languages, older C#, suppressed warnings). Removing existing checks during migration can introduce regressions for those callers. Learned from dotnet/runtime nullability.md guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Enhance Try method annotation patterns with specific recipes Clarifies the distinction between generic and non-generic Try method out parameters: use [NotNullWhen(true)] out T? for non-generic, and [MaybeNullWhen(false)] out T for generic. Also notes that Equals(object?) overrides should use [NotNullWhen(true)]. Learned from dotnet/runtime nullability.md DO guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for override covariance and IEquatable<T?> Override return types can be stricter (non-nullable) than the base even if the base returns T?. Also adds that reference types should implement IEquatable<T?> and IComparable<T?> since callers commonly pass null to Equals. Learned from dotnet/runtime PR #67198 and nullability.md guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add three-pass code review checklist for nullable changes Nullable PRs need broader review than a typical diff: (1) verify no behavior/IL changes, (2) review explicit annotation changes for correct contracts, (3) scan unchanged APIs whose implicit nullability changed when the nullable context was enabled. Learned from dotnet/runtime nullability.md Code Review Guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add link to dotnet/runtime nullable annotation guidelines This is the authoritative internal document used when annotating the .NET libraries and contains detailed guidance on gray areas, virtual methods, struct defaults, and attribute usage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strengthen null validation pitfall: NRTs are compile-time only Nullable annotations do not prevent null at runtime. Callers using older C#, other .NET languages, reflection, dynamic, or the ! operator can still pass null. Emphasizes that runtime validation on public APIs remains essential for correctness and security. Learned from community blog posts (Maarten Balliauw, JetBrains) and dotnet/runtime nullability.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Define NRT abbreviation at first use in body text Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Note that [MaybeNull] should be rare — prefer T? when possible The .NET runtime codebase has only ~7 uses of [MaybeNull]. In most cases T? is a better choice. Reserve [MaybeNull] for edge cases like AsyncLocal<T>.Value where T? would incorrectly allow setting to null when T is non-nullable. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance to use [NotNullWhen(true)] on Equals overrides When Equals(object? obj) returns true, the argument is guaranteed non-null. Annotating with [NotNullWhen(true)] lets callers skip redundant null checks after equality tests. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for widely-overridden virtual return types For virtual/abstract methods with many overrides, check whether existing overrides return null in practice. The .NET runtime annotated Object.ToString() as string? (overrides commonly return null) but Exception.Message as string (null overrides vanishingly rare). When in doubt, prefer T?. Source: dotnet/runtime nullability guidelines (ToString case study) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prefer multiple [MemberNotNull] attributes over params overload The params string[] overload of MemberNotNull is not CLS-compliant and may require #pragma warning disable. Use separate attributes for each field instead. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Warn that compiler does not flag misapplied nullable attributes Attributes like [DisallowNull] on a non-nullable parameter or [MaybeNull] on a by-value input parameter are silently ignored. Even Roslyn team members get tripped up by this. Source: dotnet/roslyn#36073 (jcouv, RikkiGibson) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Warn about false safety from nullable-oblivious libraries Unannotated dependencies produce no nullable warnings at all, giving a false sense of safety. Return values from oblivious APIs should be treated as potentially null. Source: dotnet/roslyn#44063 (jcouv), dotnet/roslyn#38941 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify Try method annotation: generic vs non-generic out params Non-generic out params: declare nullable with [NotNullWhen(true)]. Generic out params: keep non-nullable with [MaybeNullWhen(false)]. Using T? on generic params would change value-type signatures to Nullable<T>. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for unconstrained generics and LINQ null filtering For generic methods returning default on unconstrained T, use [return: MaybeNull] T instead of T? to avoid changing value-type signatures to Nullable<T>. LINQ Where(x => x != null) does not narrow T? to T. Use a WhereNotNull() helper (as Roslyn itself does) or OfType<T>(). Source: dotnet/roslyn#30953 (jcouv, roji), dotnet/roslyn#39586 (sharwell) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: var is always considered nullable by the compiler The compiler treats var as nullable regardless of the assigned expression — flow analysis determines the actual null-state. This can surprise developers who expect var to inherit non-nullability from the right-hand side. Learned from Maarten Balliauw's nullable migration blog series. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add strategy guidance: start at the center, work outward Annotating core domain models, DTOs, and shared utilities first eliminates cascading warnings across the codebase and gives the biggest return on effort. Applies to all three rollout strategies. Learned from Maarten Balliauw's JetBrains 2025 talk and blog series. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add dedicated EF Core section with schema migration warning EF Core infers column nullability from NRT annotations. Enabling NRTs can silently generate migrations that make previously nullable columns required, risking data loss. Consolidates all EF Core guidance (navigation properties, schema impact, migration review) into a dedicated section and removes the scattered EF references. Learned from Maarten Balliauw's JetBrains 2025 talk, Damir Arh's blog, and EF Core documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: warn that #nullable disable warnings is insufficient for entity files #nullable disable warnings only suppresses compiler diagnostics but leaves the annotation context active. EF Core reads annotations via reflection, so properties without ? are still treated as required, potentially altering database schema. Use #nullable disable to fully opt entity files out. Source: dotnet/efcore#33245 (confirmed by EF team member roji). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: add DbSet<T> initialization guidance DbSet properties should remain non-nullable since EF Core always initializes them. EF Core 7.0+ suppresses CS8618 automatically; older versions can use = null! or expression body => Set<T>(). Source: official EF Core NRT docs page. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: expand navigation property guidance with three approaches Replace single-sentence guidance with the three patterns from the official EF Core NRT docs: (a) non-nullable with null!, (b) nullable when code checks load state, (c) non-nullable wrapping nullable backing field that throws. Also note collection navigations must always be non-nullable (empty, never null). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: add LINQ query guidance for optional navigations EF Core translates LINQ to SQL so null navigations don't throw at runtime, but the compiler still warns. Use ! in Where/Include expressions to suppress false positives. From official EF Core docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ASP.NET Core: MVC treats non-nullable properties as [Required] When NRTs are enabled, ASP.NET Core MVC/Web API implicitly adds [Required(AllowEmptyStrings = true)] to every non-nullable reference type property in DTOs and view models. This can cause previously accepted requests to return 400 Bad Request. Controllable via SuppressImplicitRequiredAttributeForNonNullableReferenceTypes. Source: official ASP.NET Core model validation docs, aspnetcore#22656. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ASP.NET Core: System.Text.Json RespectNullableAnnotations and limits STJ (.NET 9+) can enforce NRT annotations at serialization time via RespectNullableAnnotations, but cannot enforce collection element nullability (List<string> vs List<string?> are indistinguishable in IL), top-level types, or generic properties. Document the opt-in and its limitations. Source: official STJ nullable annotations docs, aspnetcore#57234 (confirmed by eiriktsarpalis and terrajobst). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ASP.NET Core: warn about #nullable disable warnings on model files Same pitfall as EF Core: #nullable disable warnings only suppresses compiler diagnostics but leaves annotations active. MVC reads them via reflection to infer [Required], so model files not yet migrated must use #nullable disable to fully opt out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Expand rollout strategy guidance and add suppression analysis use case Add 'analyzing existing suppressions' to When to Use section. Expand the start-at-center strategy with guidance to work outward through layers and keep PRs scoped per project or layer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add helper extension methods section with WhereNotNull Add a Helper Extension Methods section with WhereNotNull for both reference and value types. This is the pattern used by the Roslyn compiler codebase to filter nulls with correct type narrowing, which .Where(x => x != null) cannot provide. Update the inline reference in Step 3 to link to the new section. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add incremental build checkpoints to NRT migration workflow Add explicit build checkpoint instructions after Steps 2-6 so the AI builds and checks warning counts after each phase rather than attempting all fixes blind. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add commit strategy guidance to NRT migration workflow Tell the AI when to commit during the migration: at each logical boundary (enable, dereference fixes, annotations, attributes, cleanup) to keep diffs reviewable and prevent lost work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add 'when to ask the user' guidance for annotation decisions Tell the AI to ask the user before changing public API contracts, deciding nullable vs required for ambiguous properties, or choosing between null checks and suppressions when design intent is unclear. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add inline anti-pattern callouts with warning markers Add prominent ❌ and ⚠️ callouts inline in Steps 3 and 4 where dangerous mistakes are most likely: using ?. to silence warnings, sprinkling !, adding ? to value types, letting warnings drive annotations, and removing existing null argument validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add decision flowchart for resolving nullable warnings Add a structured decision tree in Step 3 that guides the AI through each warning: is null valid by design? Can you prove non-null? This prevents defaulting to ! or over-annotating with ?. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add scope-based prioritization guidance to Step 3 Tell the AI to work in dependency order (core models first, then consumers) and prioritize public/protected members within each file to minimize cascading warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add verify-before-claiming guidance to validation step Tell the AI to verify annotations match design intent, check for accidental ?. operators, confirm no removed null checks, and audit ! usage before reporting the migration as complete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add conditional behavior guidance based on project type Tell the AI to tailor its approach based on whether the project is a library (public API contracts first), application (boundaries first), or test project (more liberal use of !). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Flag public API methods missing runtime null validation Tell the AI to check for missing ArgumentNullException guards on non-nullable public/protected parameters during annotation. Without runtime checks, null passed at runtime causes NRE instead of ANE. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add Purpose paragraph to match recommended SKILL.md structure Add a one-paragraph Purpose description under the top-level heading, describing the outcome of using this skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Couple notes * Add missing reference links to More Info section Add links to Nullable reference types overview, C# language reference page, and null-forgiving operator reference. These are foundational docs referenced throughout the skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add breaking changes section for library NRT annotations Warn that incorrect NRT annotations are source-breaking changes for library consumers. Covers implicit non-nullability, parameters that should be nullable, return types, and versioning guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: methods tolerating null should accept nullable params If a method handles null gracefully (returns null, returns default, returns a failure result) rather than throwing, the parameter should be T?, not T. Based on BCL conventions and NodaTime design discussions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make build/test references build-system agnostic Not all repos use dotnet build/test directly — some use build scripts, msbuild, cake, nuke, etc. Add build/test command as optional inputs, update Step 1 to detect the build system, and replace hardcoded 'dotnet build'/'dotnet test' with generic 'build'/'test' in checkpoints and validation checklist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove cake/nuke from build script examples These are uncommon enough to not warrant explicit mention. Keep the list focused on what an agent would actually encounter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Recommend WarningsAsErrors in project file instead of CLI flag <WarningsAsErrors>nullable</WarningsAsErrors> in the .csproj or Directory.Build.props permanently prevents nullable regressions, which is better than relying on a CLI flag that CI might miss. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Track and document breaking changes for public libraries When annotating a public library, the AI should create a nullable-breaking-changes.md listing all public API changes that could affect consumers (return types narrowed/widened, parameters changed, value type nullability, new guards). This file serves as a review artifact and basis for release notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add trigger phrases to skill description for discovery Add CS8602, CS8618, CS86xx, #nullable enable, annotating APIs, and null-forgiving operator keywords so agents discover this skill for a wider range of nullable-related user requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move framework-specific sections to references/ for progressive disclosure Move EF Core, ASP.NET Core, breaking changes, and helper extension method content into references/ files. SKILL.md retains one-line summaries with links so the agent loads detailed content only when relevant. Reduces SKILL.md from 218 to 194 lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add NRT readiness scanner script Add scripts/Scan-NullableReadiness.ps1, a PowerShell Core script that scans a project or solution and reports NRT migration status: project settings (Nullable, LangVersion, TargetFramework, WarningsAsErrors) and source-level counts (#nullable directives, ! operators, #pragma CS86xx suppressions). Supports -Path, -Json, and -Recurse parameters. Update SKILL.md Steps 1 and 6 to reference the script as an optional automation step. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix strict mode .Count errors in Scan-NullableReadiness.ps1 Wrap pipeline results in @() before accessing .Count to avoid 'property Count cannot be found' errors under Set-StrictMode -Version Latest when a pipeline returns a single object instead of an array. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add automated tests for Scan-NullableReadiness.ps1 Test script runs the scanner against three fixture projects: - nrt-disabled: no <Nullable>, has ! operators and #pragma suppressions - nrt-enabled: <Nullable>enable, clean code - nrt-partial: <Nullable>enable but files with #nullable disable and ! Validates JSON output matches expected NRT readiness state (19 assertions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Recommend clean builds for validation checkpoints Incremental builds only recompile changed files, hiding nullable warnings in untouched files. All build checkpoints should use clean builds (e.g., --no-incremental) to surface all warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move nullable-reference-migration to src/dotnet/ layout Align with upstream repo restructuring that moved skills and tests under src/dotnet/. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve ! operator counting by stripping comments and strings The scanner now removes block comments, line comments, verbatim strings, interpolated strings, and regular strings before counting null-forgiving operators. This eliminates false positives from ! in prose like 'Important!' or 'Value required!'. Tests updated with exact assertions and fixtures enriched with ! in comments, XML docs, and string literals to verify filtering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix misleading 'var is always nullable' wording var does not inherently introduce nullability — the inferred type carries nullability from the right-hand side expression. Updated the pitfall description to reflect this accurately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make STJ RespectNullableAnnotations limitations more explicit Clarify that the feature is opt-in, affects both serialization and deserialization, and explicitly list what it does not cover: collection elements, dictionary values, top-level types, and generic type parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add caveat that Debug.Assert is stripped from Release builds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify that required does not enforce during deserialization The required keyword is a compile-time and object-initializer constraint. It does not replace runtime validation unless the serializer explicitly supports it (e.g., STJ with [JsonRequired]). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add explicit version callouts for required, DbSet, JsonRequired Consistently tag version requirements so the skill adapts across repos targeting different .NET versions: - required (C# 11+) on all mentions - EF Core 7.0+ (.NET 7) for DbSet CS8618 suppression - [JsonRequired] (.NET 7+) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add DTO vs domain model nullability guidance DTOs should be nullable by default (they cross trust boundaries), domain models should prefer non-nullable invariants. This distinction is where migrations most often go wrong. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add gray area heuristics for parameter nullability decisions From dotnet/runtime nullability guidelines: when a parameter is neither validated nor documented for null, check usage patterns in the codebase, whether null acts as a default/nop, and whether similar methods accept null. When in doubt, prefer nullable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Never infer nullability intent from usage frequency or naming Explicit guard against AI hallucination: if intent is not clear from code or documentation, ask the user rather than guessing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strengthen serialization bypass warning in pitfalls table Reworded to anchor the rule: serializers bypass constructors, non-nullable properties on DTOs do not imply runtime safety. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Stop and ask user if lang version or TFM is insufficient for NRTs If the project targets C# 7.x or a framework that defaults below C# 8.0, explicitly inform the user what needs to change and ask whether to update or abort. Do not silently proceed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add = null! guidance for late-initialized fields in NRT skill Add recommended technique for CS8618 warnings on fields set after construction: use = null! on the field declaration instead of adding ! at every use site. Document the pattern in the CS8618 table row, the annotation decision flowchart, and the Step 3 guidance section. Motivated by exercising the skill on System.Text.RegularExpressions, where = null! reduced ! operator count from 423 to 178 compared to the original 2019 NRT annotation PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Categorize ! operators as null!/default! vs assertions in scanner Break down the ! operator count into two categories: - null!/default! initializers (safe, expected for late-initialized fields) - assertions (value! patterns that warrant scrutiny) This gives a more nuanced readiness assessment — a project with 30 null! and 5 assertions is in better shape than one with 35 assertions, even though the total is similar. Update fixtures with null!/default! patterns and add 6 new test assertions (25 total, up from 19). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pre-migration effort estimate for CS8618 warnings Count uninitialised reference-type fields and auto-properties as a rough predictor of CS8618 warnings before NRT is enabled. Excludes value types, events, const, and 'required' properties. Shown only when NRT is not yet enabled, labeled as approximate. In the corefx regex migration, CS8618 was 47% of all warnings (236 of 502), making this the single most useful pre-migration signal. Adds 3 test assertions (28 total). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add per-file ! breakdown in scanner output Show null!/default! vs assertion counts for each flagged file in both human-readable and JSON output. This helps reviewers quickly see which files have mostly safe initializers vs genuine assertions that warrant scrutiny. Example: 'RegexCompiler.cs: 31 ! (24 null!/default!, 7 assertions)' tells the reviewer only 7 uses need attention, not 31. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Show per-file uninit fields pre-migration, suppress post-migration Flag files with >5 uninitialised reference-type fields in 'Files needing attention' so the pre-migration report shows where the most CS8618 work will be. Suppress this metric when NRT is already enabled since it's no longer actionable. Also skip empty lines in per-file output when a file only triggered the uninit threshold but NRT is enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Context-aware section headings in scanner output Replace generic 'Files needing attention' with context-dependent labels: - Pre-migration (NRT not enabled): 'Migration work needed' - Post-migration with remaining directives: 'Remaining cleanup' - Post-migration clean: 'Suppression audit (Step 6)' — frames ! operators as review candidates, not problems Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove Step 6 reference from scanner output to avoid sync issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add before/after example output to scanner script header Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Advocate RespectNullableAnnotations as best practice for .NET 9+ Reframe guidance from neutral opt-in to prescriptive best practice, add motivation for why it matters, and recommend companion setting RespectRequiredConstructorParameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add zero-IL-change principle as hard rule for NRT migration Add prominent top-level principle: NRT migration must never change runtime behavior or generated IL. Missing null guards should be flagged as TODO comments (with user consent), not added inline. Update the existing null validation guidance to align. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add TFM caveat to Debug.Assert null-state guidance Debug.Assert acts as a nullable flow-analysis hint only on .NET Core 3.0+ where it has [DoesNotReturnIf(false)]. On .NET Framework/netstandard2.0 it lacks this annotation and does not affect null-state analysis. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert Debug.Assert TFM caveat The skill targets .NET Core 3.0+ where Debug.Assert has [DoesNotReturnIf(false)]. No need to document .NET Framework behavior since we don't encourage NRTs on older TFMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add return-type semantic nullability guidance to NRT skill Round-trip experiment on Dapper (51 files, 984 warnings) revealed the agent systematically removed ? from 40 public return types (OrDefault, ExecuteScalar) to satisfy the compiler, but those methods can return null by design. Added explicit guidance that return types must reflect semantic nullability, not just compiler satisfaction, with specific patterns to watch for. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strip string literals before comments in scanner Reorder the regex passes so strings are removed before comments. A string like "http://..." contains // that was previously mis-parsed as a line comment, corrupting downstream text. Add a URL-in-string test case to the nrt-enabled fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add NRT skill guidance from real-world evaluation Tested the skill against NimblePros/eShopOnWeb (ASP.NET Core + EF Core). Stripped NRT annotations from 44 files, re-annotated using the skill, and compared against the originals. Found 3 areas for improvement: ef-core.md: - Add guidance to always pair #pragma warning disable CS8618 with #pragma warning restore CS8618 to prevent suppression leakage aspnet-core.md: - Add Razor Pages [BindProperty] initialization guidance - Add collection property default initialization guidance - Call out the ?.+! anti-pattern (contradictory null handling) SKILL.md: - Add guard clause library awareness (Ardalis.GuardClauses, Dawn.Guard use [NotNull] attributes that narrow null state — avoid redundant !) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename skill to migrate-nullable-references, wire scanner tests into eval.yaml, merge main - Rename nullable-reference-migration -> migrate-nullable-references per CONTRIBUTING.md naming convention (lead with action verb) - Update YAML front-matter name field - Update scanner script and test script references to new directory name - Add scanner script eval scenario to eval.yaml using test fixtures - Merge latest main for CI/eval infrastructure updates Fixes #38 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove en-us locale from learn.microsoft.com URLs in NRT skill Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove scanner eval scenario (script tests stay in Test-ScanNullableReadiness.ps1) The scanner script scenario doesn't work as an eval scenario because the baseline run has no access to skill resources (source: files aren't resolved when skill=null). Scanner tests are validated separately via Test-ScanNullableReadiness.ps1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve eval rubric for correctness and fix code formatting Replace generic throughput rubric with 5 correctness-focused criteria: - Value-type avoidance (int, bool, DateTime) - Nullable return types for FirstOrDefault methods - Late-init field handling (_header in CsvExporter) - Interface/implementation consistency - Zero-warning build Also fix C# code formatting in setup files (namespace/class declarations were collapsed onto single lines). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve migrate-nullable-references skill routing and deduplication - Add structured USE FOR / DO NOT USE FOR / INVOKES to description - Add multi-project solution migration order note in Step 2 - Deduplicate serialization/DTO pitfall (cross-ref Step 4) - Shorten redundant ArgumentNullException pitfall row Multi-model validation: Sonnet 4.8/5, GPT-5.1 4.6/5, Haiku 4.8/5 (avg 4.73) No regressions detected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add minimal API parameter optionality guidance per halter73 review Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix contradiction in CS86xx fix guidance per eerhardt review The warning fix table recommended adding null checks/guards, contradicting the zero-runtime-behavior-change rule in line 35. Reworded to prioritize annotation-only fixes (? and !) and note that null checks change behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skill: prefer [NotNullWhen(true)] over ! at call sites When a boolean-returning helper guarantees a nullable parameter is non-null, annotating the parameter with [NotNullWhen(true)] is better than sprinkling ! at every call site. This is metadata-only (no behavior change) and gives the compiler real flow information. Based on feedback from reviewing skill-guided NRT migration output on dotnet/roslyn#58107 — the agent used ! without comment instead of annotating the helper method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove WhereNotNull helper extensions; refine skill scope - Delete helper-extensions.md (WhereNotNull was rejected as a BCL API and is only used internally in Roslyn) - Update LINQ guidance to recommend OfType<T>() instead - Add 'fixing a handful of warnings' to DO NOT USE FOR in description Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename Scan-NullableReadiness to Get-NullableReadiness per convention Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add ASP.NET Core + EF Core eval scenario for NRT migration Exercises EF Core navigation property nullability, DbSet handling, and ASP.NET model validation behavior changes when enabling NRTs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add file_not_contains assertion and file-by-file eval scenario Add file_not_contains assertion type to skill-validator, mirroring file_contains but asserting the value is absent. Includes 3 tests. Add eval scenario testing file-by-file NRT migration: agent must modify only the targeted file while leaving others unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix YAML frontmatter: use block scalar for description with colons The colons in USE FOR:/DO NOT USE FOR:/INVOKES: caused YAML parse failures in skill-validator. Use > block scalar syntax per lewing review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strengthen CS8603 return-type guidance and add return null! anti-pattern Clarify that non-nullable return type is a promise to callers. Add explicit warning against return null!/default! pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Extract nullable attributes table to optional reference Move the System.Diagnostics.CodeAnalysis attributes table from Step 5 inline content to references/nullable-attributes.md, reducing the core skill by ~500 tokens. The inline step retains a summary listing the key attribute names so the agent knows what's available and when to load the reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tweak ASP.NET/EF eval scenario prompt wording Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move skill to plugins/tests layout, add CODEOWNERS for migrate-nullable-references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: remove spoon-fed build commands, refocus rubric on skill value - Remove explicit 'Build with:' lines from eval prompts so the skill's Step 1 (discover build command) is actually exercised - Reword file-by-file rubric to test the skill's migration strategy rather than general instruction compliance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Larry Ewing <lewing@microsoft.com> |
||
|
Mark Miller
|
3aa3ddbf1c | Skill for debugging .NET Framework runtime activation issues using the activation logs added in the in-proc SxS days. (#152) | ||
|
Jared Parsons
|
8f06012483 |
Update contributing guidelines (#151)
* Update contributing guidelines Clarify evaluation criteria for third-party tools and add ownership requirement for skills. * Update CONTRIBUTING.md to remove skill owner requirement Removed requirement for every skill to have an associated owner. * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Tim Heuer <4821+timheuer@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
29033c459d | Reset plugin verisons to 0.1.0 (#145) | ||
|
Viktor Hofer
|
1cfa908e91 |
Add CODEOWNERS file with per-skill ownership (#148)
* Add CODEOWNERS file with per-skill ownership Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update .github/CODEOWNERS Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add code ownership requirements to CONTRIBUTING.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move code ownership section near top of CONTRIBUTING.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update .github/CODEOWNERS Co-authored-by: Jared Parsons <jared@paranoidcoding.org> * Update .github/CODEOWNERS Co-authored-by: Jared Parsons <jared@paranoidcoding.org> * Update .github/CODEOWNERS Co-authored-by: Jared Parsons <jared@paranoidcoding.org> * Update .github/CODEOWNERS Co-authored-by: Jared Parsons <jared@paranoidcoding.org> * Update .github/CODEOWNERS Co-authored-by: Jared Parsons <jared@paranoidcoding.org> * Update .github/CODEOWNERS * Update .github/CODEOWNERS * Update .github/CODEOWNERS Co-authored-by: Jared Parsons <jared@paranoidcoding.org> * Modify CODEOWNERS for msbuild and cleanup Updated CODEOWNERS to change 'dotnet-msbuild' to 'msbuild' and removed TODO comment. * Fix CODEOWNERS case sensitivity for TimHeuer * Fix CODEOWNERS syntax for user list --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jared Parsons <jared@paranoidcoding.org> |
||
|
Cameron Aavik
|
4d11e331dd |
Add microbenchmarking skill (#63)
* Add microbenchmarking skill * Add eval.yaml * Merge in latest changes + apply review comments * Use stronger wording for reference file loading * Remove DoNotParallelize * Move skill and tests to new location |
||
|
Viktor Hofer
|
4d484a18ce |
Move src/dotnet content into plugins/ and tests/ (#144)
Move skills from src/dotnet/skills/ to plugins/dotnet/skills/ and tests from src/dotnet/tests/ to tests/dotnet/, then remove the now-empty src/ directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
8023c59a07 |
Fix remaining relative fixture paths in analyzing-dotnet-performance eval
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
0d4396e1f9 |
Fix relative fixture path in analyzing-dotnet-performance eval
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
2238b1c4d5 |
Update repository layout in CONTRIBUTING.md
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
24fa12656b |
Remove .mcp.json and mcpServers from plugin.json
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
6a60761c7d |
Group agentic-workflows content under dotnet-msbuild subfolder
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
7115c467d7 |
Move build.ps1 to agentic-workflows and update paths
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
2dacda5600 |
Remove AGENTS.md and README.md from dotnet-msbuild plugin
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
077ca9155f |
Remove prompts folder and distribution templates section
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
1e44eef4f1 |
Remove copilot-extension folder and all references
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
7d0a329aee |
Move agentic-workflows to repo root
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
86cd604ee6 | Fix missing agents from dotnet plugin | ||
|
Viktor Hofer
|
5005d751ac |
Remove validate-agents and validate-skills workflows
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
6e2ad53e88 | Move plugin tests out of plugins folder into a root tests/ folder | ||
|
Viktor Hofer
|
0b249be6b6 | Rename from components to plugins | ||
|
Viktor Hofer
|
250b1c2bbc | Rename src/ to plugins/ folder | ||
|
Dan Moseley
|
bebcb57e94 |
Thread.Abort migration skill (#126)
* Thread.Abort migration skill built with a research and iterative evaluation approach * Move thread-abort-migration skill to src/dotnet/ structure Move SKILL.md and eval.yaml to match the current repository layout where skills live under src/<component>/skills/ and tests under src/<component>/tests/. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix fragile assertion in thread-abort-migration negative test The 'should not migrate' scenario used output_contains with the exact string 'no migration', which is too brittle — the agent may phrase this many ways. Replace with output_matches using a flexible regex that accepts 'no migration', 'not require migration', 'doesn't need migration', etc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add negative activation guidance to skill description Add 'Do not use' clause for code that only uses Thread.Join/Sleep/Start without any abort or interrupt patterns, matching the convention used by other skills (e.g. dotnet-pinvoke, msbuild-antipatterns). This prevents unnecessary skill loading for general Thread usage that needs no migration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add ASP.NET Response.End/Redirect eval scenario Covers the common pattern where ASP.NET Framework internally calls Thread.Abort via Response.End() and Response.Redirect(url, true). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add More Info section with official docs links Links to the .NET 6 breaking change doc, cooperative cancellation guide, CancellationTokenSource API, SYSLIB0006/0046 warnings, and Task.WaitAsync reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix redundant '(6+/8+)' to '(6+)' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix SYSLIB0006 formatting consistency Backtick-format the one remaining unformatted instance in markdown body. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add commit strategy guidance to workflow section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Broaden negative test assertion regex Also match 'no changes needed', 'no changes required', 'already compatible', 'already works', 'already supported'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rephrase negative test prompt to trigger skill activation Mention Thread.Abort explicitly in the prompt so the skill activates, then the rubric verifies the skill correctly identifies no migration is needed. Prevents validator failure from non-activation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove misleading Thread.Join → Task.Wait guidance Drop the Thread.Join replacement from the blocking call interruption bullet. Thread.Join waits on a Thread, not a Task, and the one-line guidance was ambiguous about which thread is being discussed. Per feedback from @marklio. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rewrite ResetAbort guidance to focus on work unit decomposition CancellationTokenSource cannot be reset once cancelled. Reframe the guidance around breaking up abortable units of work so the processing loop can continue to the next unit, and creating a new CTS per unit. Per feedback from @marklio. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix conflicting assertion in ASP.NET eval scenario The rubric requires explaining that Response.End internally calls Thread.Abort, but output_not_contains 'Thread.Abort' would fail a correct explanation. Change to output_not_matches with a pattern that only forbids calling Thread.Abort(), not mentioning it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Loosen ASP.NET redirect assertion A correct ASP.NET Core migration may use Redirect(), RedirectToAction(), or return Redirect() rather than Response.Redirect. Accept any form. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Task.Delay timeout cleanup guidance in Step 3 Pass the CTS token to Task.Delay so the timer is cleaned up when the source is cancelled, and note the disposal behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Note SYSLIB0046 obsolescence for Thread.Interrupt in cleanup table Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Mark Miller (CLR) <markmil@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Mukund Raghav Sharma (Moko)
|
6dfbf054b4 |
Add optimizing-ef-core-queries skill (+8.7% eval, near-miss) (#90)
* Add optimizing-ef-core-queries skill (+8.7% eval, near-miss) Teaches EF Core query optimization: N+1 detection and fixes, AsNoTracking for read-only queries, compiled queries for hot paths, AsSplitQuery for cartesian explosion, and common query traps (client eval, ToList before Where). Eval results: +8.7% improvement (threshold: 10%, near-miss) Includes eval.yaml with N+1 fix scenario + negative test. * Replace N+1 scenario with ExecuteUpdateAsync/ExecuteDeleteAsync bulk ops * Update src/dotnet/skills/optimizing-ef-core-queries/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Jan Krivanek
|
00b9fb17ef |
[Infra] Add PoC of overfitting judging (#140)
* Add overfitting judging * Add the design doc * Add async I/O * Add overfit detection description |
||
|
Viktor Hofer
|
dcafe88a5b |
Fix running SkillValidator on Windows locally (#138)
The NormalizeDirectory function + the quote resulted in the quote getting escaped: `\"`. Use NormalizePath instead as we don't need the trailing slash. |
||
|
Jan Krivanek
|
c595feb95d | Support skill negative tests (#139) | ||
|
Viktor Hofer
|
a077eda88a |
Remove shell tool restrictions from agentic workflows (#136)
The shell tool restrictions (no compound commands, env dotnet workaround) are no longer needed as of gh aw 0.56.0 which fixed these limitations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Fred Silberberg
|
c938773ead | Fix typo in README.md (#132) | ||
|
Elinor Fung
|
3e4ba910f0 | Fix quotes in RunArguments for SkillValidator (#134) | ||
|
Steve Pfister
|
b1b52e6c15 | Add dump-collect skill for modern .NET (#103) | ||
|
Viktor Hofer
|
622cc7220c |
Increase timeout for evaluation job to 60 minutes (#129)
The msbuild component is currently the biggest one and takes just slightly over 30 minutes. Let's bump the timeout to 60 as that's still low enough. |
||
|
Jan Krivanek
|
a212d0c6bf |
Add build organization skill eval (#123)
* Add build organization skill eval * Address PR review comments: fix async elision, leading blank line in sln, missing using Xunit |
||
|
Jan Krivanek
|
b0f531b642 |
Add build perf baseline eval tests (#128)
* Add build perf baseline eval tests * Reflect PR comments * Do not reveal test requirements |
||
|
Art Leonard
|
0cee0843ca |
Add .NET performance analysis skill and optimizing agent (#6)
* Add .NET deep performance fixer agent and performance patterns skill Introduce a dotnet-performance-optimizer agent and the dotnet-performance-patterns skill with reference guides covering: - Async patterns - Collections and LINQ - Critical performance patterns - I/O and serialization - Memory and strings - Regex patterns * Align agent and skill performance reporting workflow - Add two-pass analysis methodology (Pass 1: LLM-only, Pass 2: skill-based) - Consolidate skill references to match actual reference files - Remove references to non-existent skills (dotnet-jit-optimization, dotnet-async-patterns, dotnet-sync-primitives) - Update pattern count to 85+ to reflect current reference corpus * Update performance patterns skill with expanded reference coverage - Add structural-patterns.md reference file - Expand collections-and-linq, memory-and-strings, regex patterns references - Update critical-patterns with refined rules - Expand SKILL.md with additional workflow guidance * Add missing structural-patterns.md reference to agent skill list * Add non-deterministic results disclaimer to agent and skill output * Trim agent per skill authoring best practices Apply recommendations from platform.claude.com best practices: - Remove sections Claude already knows (Core Expertise, Edge Cases, Key Principles, When to Ask, Tone) - these waste context tokens - Remove skill internal reference file listing (skill handles its own progressive disclosure) - Condense 138 lines to 55 lines (-60%) - Keep only what Claude needs: two-pass workflow, output format, skill reference, escalation guidance * Trim SKILL.md per skill authoring best practices Apply recommendations from platform.claude.com best practices: - Remove Purpose section (redundant with description frontmatter) - Remove When to Use (redundant with description triggers) - Consolidate signal table (11 rows → 5, merging same-reference rows) - Remove 50-line checklist example template (keep the rule, not the example) - Remove 25-line Recipe Completeness Verification (redundant with gate) - Remove 40-line Output Format example (Claude knows markdown) - Trim Validation from 8 items + gate to 5 essential items - Trim Common Pitfalls from 3-column to 2-column - Condense Steps 5+6 into single Step 5 - 311 lines → 134 lines (-57%) * Trim reference files per Claude skill authoring best practices Apply 'Claude is already smart' principle to reference files: - Remove 6 patterns Claude already knows (Interlocked, SemaphoreSlim dup, Array.Empty, Any() vs Count(), Enum.HasFlag, Random.Shared) - Remove IEquatable<T> duplicate (kept in structural-patterns.md) - Remove 'Why a Separate File' meta-commentary from structural-patterns.md - Remove all horizontal rules between patterns - Remove explanatory prose and obvious inline comments - Preserve all Detection sections, scan recipes, and Impact statements Total: 1,819 → 1,372 lines (-25%) * Rename skill and agent to gerund form per naming conventions - dotnet-performance-patterns → analyzing-dotnet-performance - dotnet-performance-optimizer → optimizing-dotnet-performance - Update all cross-references in frontmatter and agent skill list * Rewrite descriptions per effective description guidelines - Agent: Replace second-person trigger phrases and verbose examples with concise third-person description stating what it does and when to use it - Skill: Add specificity (85+ patterns, tiered severity) and clearer triggers - Both now follow: third person, specific key terms, what + when format * Refine performance report structure and findings format for clarity and conciseness * Remove outdated reference link from SKILL.md for clarity * Remove further reading section from SKILL.md for conciseness * Add compound allocation check to performance analysis workflow * Add performance analysis scenarios for critical patterns, collections, I/O, and struct issues * Address PR #6 review feedback from Stephen Toub Remove 15 patterns that are correctness-only, redundant, or not actionable: - async void, lock+async, Async All the Way Down, NonBacktracking, Regex timeouts, Cache Regex, Budget Compiled Regex, Seal Classes stub, ConfigureAwait(false), Parallel.ForEachAsync, async lambda Action, C#10 Interpolation Handlers, Span-Based Comparison, Utf8JsonWriter, JsonSerializerContext for AOT Clarify 3 patterns where rationale was unclear: - IEquatable<T>: explain boxing via EqualityComparer<T>.Default - IsMatch guard: explain Replace always allocates even on no-match - Chained strings: split into 3 distinct anti-pattern/fix pairs Rework 3 patterns per feedback: - CollectionsMarshal.AsSpan: qualified to ultra-hot-path-only - LINQ guardrail: moved to Common Pitfalls table - params overloads: added params ReadOnlySpan<T> as .NET 9+ alternative Fix nits: - Title: arrays → substring for slicing pattern - Remove try/finally from ArrayPool example - Qualify impact statement on boxing - Remove numbered headings for consistency across reference files - Add unsafe code pitfall to Common Pitfalls - Update pattern count 85→70, critical count 24→17 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move skill, agent, and tests to src/dotnet/ component layout Restructure to match upstream directory conventions: - skills/ → src/dotnet/skills/ - agents/ → src/dotnet/agents/ - skill tests → src/dotnet/tests/ - Update eval.yaml fixture source paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix eval.yaml source paths to resolve relative to skill directory The validator resolves source paths via join(skillPath, source), so fixture references need ../../tests/ prefix from the skill dir. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove validator results from tracking and add to .gitignore Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Replace synthetic eval fixtures with Humanizer-derived needle-in-haystack test suite Replace 4 synthetic Contoso fixtures with 11 realistic test fixtures modeled on patterns from the Humanizer library performance analysis (Runs A-J). 10 eval scenarios with 51 rubric items (up from 4 scenarios / 19 rubric items): - Scenarios 1-9: patterns the skill outperformed naked model on (J vs B) - Scenario 10: baseline patterns J missed (Span inconsistencies, unsealed classes) Each fixture is 240-380 lines with ~80% clean haystack code surrounding the anti-pattern needles, including decoys, positive findings, and correct patterns as contrast. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Split scenario 10 into two focused scenarios for truncation and unsealed classes The combined scenario was timing out at 120s with 2 files and 6 rubric items. Split into: - Scenario 10: Span inconsistencies and compound method chains (Truncation.cs, 3 rubric items) - Scenario 11: Unsealed leaf classes and locale hierarchy (Locales.cs, 3 rubric items) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Increase timeout to 180s for scenarios 7 and 10 These scenarios consistently timed out at 120s before the agent could synthesize findings, causing weak/negative improvement scores despite the skill detecting correct patterns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Increase timeout to 180s for scenarios 4, 8, 9 All three timed out at 120s — agent spent time loading skill references and running grep recipes, never reaching synthesis before cutoff. Scenarios 7 and 10 already at 180s from prior fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tune SKILL.md for sandboxed eval and add results report SKILL.md changes: - Add fallback for missing reference files (skip filesystem search) - Inline core scan recipes so agent works without reference files - Add 'read small files first' guidance to reduce grep overhead - Update validation checklist for reference-optional workflow Results: 73.7% overall improvement, 11/11 scenarios strong (≥60%) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Addressing @danmoseley feedback * Update src/dotnet/skills/analyzing-dotnet-performance/references/critical-patterns.md Co-authored-by: Dan Moseley <danmose@microsoft.com> * Address PR #6 review feedback: skill docs and fixture fixes Skill/reference changes: - Remove CollectionsMarshal.AsSpan section (too much churn per stephentoub) - Remove startup/initialization as examples of non-perf-sensitive code - Split Contains into own grep with false-positive note - Clarify 'exact counts' means grep match counts - GeneratedRegex: ALWAYS for static patterns, always beneficial - Rewrite regex engine section: static->GeneratedRegex, never remove NonBacktracking - Sealed class grep catches protected and file classes - StringComparison grep uses proper ERE, catches variable args Test fixture fixes: - Fix invalid regex character class in inflector-and-regex.cs - Fix Hungarian converter cache key (use original number) - Add truncation guard for truncationString >= length - Use explicit ordered array instead of dictionary keys for Farsi/Kurdish - Preserve negative sign in ToMetric null-symbol fallback - Remove stray double semicolon Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Dan Moseley <danmose@microsoft.com> |
||
|
Jan Krivanek
|
9d3f6e0801 |
Improve binlog failure analysis skill (#121)
* Improve the skill * Update the workflows * Increase timeout * Further improve the skill * Forgotten change |
||
|
Jan Krivanek
|
e381a6afd2 | Fix MCP discovery (#120) | ||
|
Viktor Hofer
|
5f95a5c8da |
Document workflow trigger commands in agentic-workflows README (#118)
* Document workflow trigger commands in agentic-workflows README Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Pin binlog-mcp container image to immutable digest Address Copilot review comment about supply-chain risk by pinning the container image to its SHA256 digest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Enhance build failure analysis and performance audit workflows: add workflow_dispatch trigger, update instructions, and improve binlog-mcp configuration * Update agentic workflows README with accurate triggers - Fix table to reflect actual workflow triggers from frontmatter - build-perf-audit uses schedule/workflow_dispatch, not a slash command - build-failure-analysis also supports workflow_dispatch - Reorder columns: Workflow, Description, Trigger Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Jan Krivanek
|
e2da8038d4 | Add timeout info to dashboard (#119) | ||
|
Jan Krivanek
|
a9cf756c68 |
[Infra] Increase + Capture timeouts (C#) (#117)
* Increase and capture timeouts * Fix timeout checking * Fix gitignore * Improve the timeout display info * Fix timeout indicator position in markdown table Move the timeout indicator after the '/5' denominator so scores render as '1.0/5 timeout' instead of '1.0 timeout/5'. |
||
|
Viktor Hofer
|
c7c74daf8e |
Clean up dotnet-msbuild: unify lock files, compact skill, add .gitattributes (#116)
* Clean up dotnet-msbuild: unify lock files, compact skill, add .gitattributes - Add .gitattributes with * text=auto eol=lf for consistent line endings - Unify compiled knowledge lock file names across copilot-extension and agentic-workflows (build-errors, performance, style-and-modernization) - Deduplicate KnowledgeMap in build.ps1 into shared KnowledgeGroups - Compact directory-build-organization skill (16K -> 8K chars) so it fits within compiled bundle limits; move detailed examples to references/ - Remove broken links to non-existent docs/copilot-extension-design.md - Regenerate all lock files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add repo-level AGENTS.md with component build instructions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Inline references/ content during knowledge compilation Update Read-Skill in build.ps1 to resolve [text](references/*.md) links and replace them with the referenced file content. This ensures compiled lock files include the full reference material alongside the SKILL.md content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update agentic workflows to leverage compiled knowledge - build-failure-analysis: add binlog-mcp tool usage, bin/obj clash and generated file checks, common error categories - build-perf-audit: add bottleneck classification, concrete optimization recommendations (MSBuild Server, ArtifactsPath, graph build), incremental build health check - msbuild-pr-review: reference AP codes from anti-pattern catalog, add Central Package Management and Directory.Build centralization checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Change agentic workflow triggers to comment-based invocation - build-failure-analysis: /analyze-build-failure - build-perf-audit: /audit-build-perf - msbuild-pr-review: /review-msbuild Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
aef6dd634f |
Clean up MSBuild component: remove deleted skill references, update workflows (#115)
* Clean up MSBuild component: inline shared docs, remove deleted skills from README - Inline targetframework-props-evaluation.md content into msbuild-antipatterns AP-21 - Update README.md to reflect only skills that exist on disk - Remove references to deleted skills (common-build-errors, nuget-restore-failures, sdk-workload-resolution, multitarget-tfm-issues, sourcegen-analyzer-failures, build-caching, msbuild-style-guide, multithreaded-task-migration) - Include all pending branch changes (skill deletions, agent updates, shared doc removal) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update agentic workflows and regenerate compiled lock files - Remove all dangling references to deleted skills across SKILL.md files, agent files, copilot extension README, and build.js - Add build-errors to copilot-extension knowledge map in build.js (sourced from binlog-failure-analysis) - Update msbuild-pr-review.md with AP-21 and AP-19/AP-20 checks - Update msbuild.agent.md triage table and skill lists - Regenerate all .lock.md files via build.js Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review comments: fix CRLF frontmatter stripping, update README, add tool-free knowledge source - Fix readSkill() regex to handle CRLF line endings, preventing YAML frontmatter from leaking into compiled *.lock.md knowledge bundles - Add check-bin-obj-clash to copilot-extension build-errors knowledge so the bundle includes tool-free guidance alongside binlog-failure-analysis - Update copilot-extension README project structure to reflect actual filenames (*.lock.md), include domain-check.js, and fix source annotations - Regenerate all compiled knowledge bundles Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
cfc22971e1 |
Port skill-validator from TypeScript to .NET C# (#106)
* Port skill-validator from TypeScript to .NET C# Replace the TypeScript skill-validator with a .NET 10 C# implementation. Uses System.CommandLine for CLI parsing, GitHub.Copilot.SDK for Copilot communication, YamlDotNet for YAML, and xUnit v3 with MTP v2 for testing. All 135 unit tests ported and passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update CI workflows for .NET skill-validator Replace Node.js setup and npm commands with dotnet build/test/publish. Use global.json for SDK version instead of hardcoded dotnet-version. Publish self-contained binary for evaluate runners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix permission denied for Copilot SDK native binary in CI Restore execute permissions on the native copilot binary after artifact download, which strips file permissions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * nit updates * Apply upstream changes: --verdict-warn-only, safeDirName, fix benches Port three origin/main commits to the C# skill-validator and workflow: - Add --verdict-warn-only flag replacing --strict; verdict failures (threshold, completion, activation) become warnings while execution errors and --require-evals missing_eval failures remain fatal. - Add FailureKind tracking to SkillVerdict and Comparator. - Add SafeDirName sanitizer for skill directory names in Reporter. - Add per-skill verdict.json generation in JSON reporter. - Update evaluation.yml: remove continue-on-error, use --verdict-warn-only, change default infra runs to 2, skip comment-on-pr on cancellation, use ResultsFile for benchmark data generation. - Update generate-benchmark-data.ps1 to read verdicts from results.json. - Update README with new CLI flags and CI example. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update CONTRIBUTING.md for .NET skill-validator Replace Node.js/npm prerequisites and commands with .NET 10 SDK equivalents (dotnet run). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update test commands in CONTRIBUTING.md to use .csproj file * Add RunArguments for default results-dir and verbose logging - Set RunArguments in SkillValidator.csproj using MSBuild NormalizeDirectory to pass --results-dir pointing to artifacts/TestResults/skill-validator/ - Log configured results dir at startup when --verbose is enabled Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix path * Fix AgentRunner: capture tool completion events and implement permission checking - Add ToolExecutionCompleteEvent handler to capture Success/Result data from tool completions, fixing timeline showing all tools as FAIL - Add ToolExecutionStartEvent arguments capture for timeline detail - Implement CheckPermission with path extraction from PermissionRequest ExtensionData, replacing the stub that always returned true - Update Judge and PairwiseJudge to pass PermissionRequest directly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CheckPermission tests for cross-platform CI (Linux) Use Path.GetTempPath() and Path.Combine() instead of hardcoded Windows paths so tests pass on both Windows and Linux CI runners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Capture skills & tokens usages * Fix thread safety, resource leaks, and null safety in skill-validator - Use ConcurrentBag and SemaphoreSlim with double-check locking for _workDirs and _sharedClient in AgentRunner (race conditions under parallel runs/scenarios) - Make Spinner IDisposable; protect _frame/_message/_active with lock and Interlocked; use 'using var' at call site - Use 'using var' for timeout Timers in Judge and PairwiseJudge to prevent leaks on exception paths - Replace perRunPairwise[0] fallback with FirstOrDefault() to avoid IndexOutOfRangeException when list is empty Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update eng/skill-validator/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Address PR review feedback - Add per-type required field validation in ParseAssertion (e.g. file_contains requires both path and value, output_matches requires pattern) - Fix JsonDocument leaks in LlmJson: use 'using' and Clone() on RootElement in both ParseLlmJson and TryParseJson - Pin Microsoft.Extensions.FileSystemGlobbing to 10.0.0 instead of floating 10.* for reproducible builds - Fix README Development section to reference SkillValidator.slnx instead of .sln Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |