Add .NET refactoring and breaking-change skills

.github/CODEOWNERS

@@ -12,6 +12,10 @@
 # dotnet (common everyday C#/.NET)
 /plugins/dotnet/skills/setup-local-sdk/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
 /tests/dotnet/setup-local-sdk/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
+/plugins/dotnet/skills/csharp-refactoring/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
+/tests/dotnet/csharp-refactoring/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
+/plugins/dotnet/skills/dotnet-breaking-changes/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
+/tests/dotnet/dotnet-breaking-changes/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
 /plugins/dotnet/lsp.json @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
 
 # dotnet-advanced (advanced .NET development skills)

eng/known-domains.txt

@@ -42,6 +42,7 @@ github.com/dotnet/msbuild
 github.com/dotnet/roslyn
 github.com/dotnet/runtime
 github.com/dotnet/sdk
+github.com/dotnet/skills
 github.com/dotnet/templating
 github.com/microsoft/cswin32
 github.com/microsoft/cswinrt

plugins/dotnet/README.md

@@ -17,4 +17,6 @@ Prerequisites:
 
 ## Skills
 
+- [csharp-refactoring](skills/csharp-refactoring/SKILL.md)
+- [dotnet-breaking-changes](skills/dotnet-breaking-changes/SKILL.md)
 - [setup-local-sdk](skills/setup-local-sdk/SKILL.md)

plugins/dotnet/skills/csharp-refactoring/SKILL.md

@@ -0,0 +1,85 @@
+---
+name: csharp-refactoring
+description: "Performs safe, behavior-preserving refactoring of C#/.NET code, verified with build, tests, and analyzers. USE FOR: any request to rename, move, extract, split, modernize, or otherwise restructure C# code without changing behavior, including small requests like 'rename X to Y': rename a symbol/type/file across a solution; move a type or static members to another file/namespace/project; extract a method, interface, or base class; pull members up; inline a method or local; split a large class/file; consolidate or de-duplicate copy-pasted code; sync namespaces to folders; modernize to current C# idioms (file-scoped namespaces, primary constructors, collection expressions, target-typed new, pattern matching); or enable nullable reference annotations. DO NOT USE FOR: adding features, fixing bugs, writing new tests, upgrading frameworks or NuGet versions (use dotnet-upgrade), or formatting-only passes (use dotnet format)."
+license: MIT
+---
+
+# C# Refactoring (behavior-preserving)
+
+A refactor changes **structure**, never observable **behavior**. Do the edit with binding-aware tools,
+then confirm behavior held with a build + the relevant tests. Keep the effort proportional to the change:
+a one-line local rename does not need the ceremony a public multi-targeted change does.
+
+## First, is this actually a refactor?
+
+The most valuable thing this skill does is *not* restructure code you were told to restructure — it is
+catching a request that is **not** behavior-preserving before you run it through a refactor's contract.
+When the request changes results, decline the refactor framing and handle it honestly:
+
+- **Framework / NuGet version bump** → not a refactor; redirect to the `dotnet-upgrade` skills.
+- **New feature** (e.g. add a pricing tier, a flag, an endpoint) → a feature, not a refactor. If asked,
+  build it as a feature *with its own tests*; don't claim behavior is preserved.
+- **Bug fix or "simplification" that changes output** (e.g. always charge shipping, bump a discount) →
+  a behavior **change**. It is a legitimate task — do it as an explicit, tested change and update the
+  tests that lock in the new behavior — but keep it **separate** from any refactor and never label it
+  behavior-preserving. Do not stall or report "nothing to change."
+- **A rename/move with a behavior tweak smuggled in** ("rename X, and while you're there bump the rate")
+  → split it: do the rename as a behavior-preserving refactor, and treat the tweak as its own tested
+  change, or flag it and defer.
+
+Only when the request is genuinely structure-only do you proceed as a refactor.
+
+## Rename / move by bindings, not text
+
+The #1 way a "rename" silently corrupts code is editing textual matches (comments, strings, unrelated
+overloads) instead of real **bindings**. Find every binding reference first, then edit semantically. Use
+the strongest tool available: an IDE/Roslyn workspace refactoring, then the C# LSP the
+[`dotnet` plugin declares](https://github.com/dotnet/skills/blob/main/plugins/dotnet/lsp.json)
+(`findReferences`, `goToDefinition`, `incomingCalls`, `rename` code action), then analyzer code-fixes /
+Roslynator, then compiler-validated edits (edit the true bindings, rebuild, let the compiler flag misses).
+Plain find/replace only when scope is provably tiny and every hit is verified. Include **every** `partial`
+declaration, and edit the generator input, never generated (`*.g.cs`) output.
+
+For the operation → Roslyn-provider mapping and representative PRs, see
+[references/operation-catalog.md](references/operation-catalog.md).
+
+## Verify proportionally
+
+Confirm behavior is preserved after the edit — scaled to blast radius, not a fixed ceremony:
+
+- **Local / private** (method-local or `private` member, one file, single target framework, no public
+  surface, no `partial`/generated/`#if`): build once and run the **relevant** tests once after the edit.
+  If the tree is already known-green, don't burn a second full "before" baseline — rely on the post-edit
+  gate. Let the compiler catch missed references.
+- **Cross-boundary** (public/shipped symbol, multi-targeted project, `#if`/platform branches, or
+  `partial`/generated code): build/test **each** target framework (a green default build can hide a break
+  on another TFM), and run the hazards check below.
+
+Use the repo's own build/test workflow when it documents one (`README`/`CONTRIBUTING`, `build.*`, `eng/`,
+`global.json`, `.github/workflows`); its instructions win over any generic command. Otherwise:
+
+```bash
+dotnet build   # 0 errors
+dotnet test    # stays green; same pass count as before
+git restore .  # if the gate fails, revert THIS step and reassess
+```
+
+One operation per step; never mix a refactor and a behavior change in the same step. On red, revert — a
+refactor that changes behavior is a bug, not a refactor.
+
+## Cross-boundary hazards (only when it touches a boundary)
+
+If — and only if — the change touches a **public** symbol, a **multi-targeted** project, or
+`partial`/generated code, some breaks won't show up as a failing test. Search the repo for the surface
+that governs the symbol (don't assume): the public-API gate (`PublicAPI.Shipped/Unshipped.txt` for
+PublicApiAnalyzers, and/or `ApiCompat`/`<EnablePackageValidation>` — not interchangeable),
+`<TargetFrameworks>`/`#if` branches, and `InternalsVisibleTo`. Move a public type via a `[TypeForwardedTo]`
+forwarder; a *rename* needs an `[Obsolete]` shim, not a forwarder. For depth on any of these surfaces,
+load the companion **dotnet-breaking-changes** skill. For a provably local/private change, skip this.
+
+## Stop and ask when
+
+- The baseline is already red (you can't prove you preserved behavior).
+- A public/shipped API would change and there is no forwarder/shim path and no analyzer/ApiCompat gate.
+- Equivalence depends on runtime behavior tests don't cover (reflection, DI, serialization, `dynamic`,
+  P/Invoke) — flag it.

plugins/dotnet/skills/csharp-refactoring/references/operation-catalog.md

@@ -0,0 +1,37 @@
+# C# refactoring operation catalog (detail)
+
+Load this when you need the full operation taxonomy or the Roslyn provider each maps to.
+The core SKILL.md keeps only the short operation list; this file is the depth.
+
+## Contents
+
+- Operation catalog (provider, recurrence, representative PR)
+- "Modernize" is defined by the repo's analyzer config
+
+## Operation catalog
+
+Canonical operations, aligned with Roslyn's own IDE refactoring providers
+(`src/Features/CSharp/Portable/CodeRefactorings`). The provider column is informational — a
+headless CLI agent usually cannot invoke these IDE code actions directly, so apply them via the fallback
+ladder in SKILL.md.
+
+| Operation | Roslyn provider / tool | Recurrence | Representative real PR |
+|-----------|------------------------|-----------:|------------------------|
+| **Rename** symbol / type / file | rename engine (`Renamer`) | high | _runtime_ "Rename `DISABLE_CROSSGEN`"; _sdk_ "Rename `dnup` to `dotnetup`" |
+| **Move** type / member / file | `MoveType`, `MoveStaticMembers` | high | _runtime_ "Move DSA tests into System.Security.Cryptography"; _sdk_ "Move SDK task unit test projects from src/ to test/" |
+| **Consolidate / de-duplicate** | analyzer-assisted | high | _runtime_ "Consolidate ComWrappers implementation across platforms" |
+| **Modernize / simplify** idioms | `UseExplicitOrImplicitType`, `UseRecursivePatterns`, `ConvertLocalFunctionToMethod`, `AddAwait` | high | _roslyn_ "Simplify lots of redundant code in code fix providers" |
+| **Split** large class / file / assembly | extract + move | medium | _roslyn_ "Split `FeatureSwitchManager`"; _runtime_ "Move RPC contracts to a separate assembly" |
+| **Extract** method / class / interface | `ExtractClass`, extract-method, extract-interface | medium | _runtime_ "Extract `ManifestBuilder` and `EventListener`" |
+| **Enable nullable** annotations | `EnableNullable` | medium | _runtime_ "[Group 4] Enable nullable annotations for `Microsoft.Extensions.Logging.EventSource`" |
+| **Inline** method / local / constant | `InlineMethod`, `InlineTemporary` | lower | _runtime_ "Refactor `UInt128` division" |
+| **Pull up / push down** member | `PullMemberUp` | tail | move members between a type and its base/interface |
+| **Sync namespace** to folder | `SyncNamespace` | tail | align `namespace` with folder layout after a move |
+
+## "Modernize" is defined by the repo's analyzer config, not personal taste
+
+These repos drive idiom rules through `.editorconfig` + `EnforceOnBuild` (roslyn #49995 "Set
+EnforceOnBuild values for code style analyzers"). Apply the fixes the repo's own analyzers request
+(IDE00xx / CAxxxx) via `dotnet format` / code-fixes; don't impose a style it hasn't opted into, and don't
+blanket-suppress diagnostics to make an edit "pass." For _adopting_ nullable annotations specifically, see
+`dotnet-upgrade/migrate-nullable-references`.

plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md

@@ -0,0 +1,98 @@
+---
+name: dotnet-breaking-changes
+description: >
+  Keep the observable .NET/C# contract intact when editing: additions count too, and the
+  contract is wider than the file you are editing. Covers the public API surface and the
+  DIFFERENT gates repos use for it (PublicApiAnalyzers vs ApiCompat/package validation),
+  nullable/trimming/AOT annotations as API, multi-targeting and #if branches (behavior per
+  target framework), source-generated and partial code, and InternalsVisibleTo.
+  USE FOR: any edit to a shipped library/NuGet package or cross-assembly/multi-targeted code —
+  adding a public/protected member, overload, or target framework; widening what a public method
+  accepts or returns; renaming, moving, or removing a member; changing a signature, nullability,
+  or attribute; touching a partial or source-generated type; or answering "is this a breaking
+  change?". Applies to features, fixes, and refactors alike.
+  DO NOT USE FOR: framework/SDK/NuGet upgrades (use dotnet-upgrade skills), pure formatting, or a
+  single-target private app with no public/cross-assembly surface.
+license: MIT
+---
+
+# .NET breaking changes: the contract is wider than the file
+
+When you edit .NET code, the observable contract is usually **larger than the snippet in front of you**.
+A change that compiles and keeps tests green can still break a downstream consumer, another target
+framework, a friend assembly, or regenerate away on the next build. This applies to a feature or a bug
+fix as much as a refactor, and to **additions** — a new public member, overload, or target framework — as
+much as removals: added surface is a permanent contract obligation, and added behavior on a multi-targeted
+or partial type must stay correct on every target and survive the next regeneration. (For the
+behavior-preserving refactoring *process*, use the `csharp-refactoring` skill; this skill is the
+compatibility knowledge it — and any other change — taps into.)
+
+## Inspect first — find which surfaces this repo actually has
+
+The most valuable move is to **look before you leap**: find which hidden surfaces exist here, then check
+only those. Don't recite these or assume — the markers present dictate the plan; markers absent tell you a
+surface is not in play.
+
+```bash
+# Public-API gate (which one? they are NOT interchangeable)
+git ls-files "**/PublicAPI.Shipped.txt" "**/PublicAPI.Unshipped.txt"                 # PublicApiAnalyzers
+grep -rl "EnablePackageValidation\|ApiCompat" --include=*.props --include=*.targets --include=*.csproj .
+# Multi-targeting and conditional code
+grep -rl "<TargetFrameworks>" --include=*.csproj --include=*.props . ; grep -rn "#if " --include=*.cs .
+# Generated / partial code
+git ls-files "*.g.cs" "*.generated.cs" ; grep -rln "partial class\|partial record\|partial struct" --include=*.cs .
+# Friend assemblies
+grep -rn "InternalsVisibleTo" --include=*.cs --include=*.csproj --include=*.props .
+```
+
+Then read only the reference(s) for the surfaces you actually found.
+
+## The hidden surfaces (depth in references/)
+
+Public API (1) is the **default** surface to check on any library edit; the other three are
+**conditional** — pursue them only when the inspect-first markers above show they are in play.
+
+1. **Public API.** Renaming/moving/removing/re-signing a public member — or changing nullability, a
+   generic constraint, or a trimming/AOT attribute — is a breaking change a green test run will not catch.
+   Repos gate this **two non-interchangeable ways**: source-level (`PublicApiAnalyzers` + `PublicAPI.*.txt`,
+   which you maintain) and binary/package-level (`ApiCompat` / `<EnablePackageValidation>`). Respect the
+   gate that exists; if none exists, review the surface by hand and flag it — do **not** bolt on analyzer
+   infrastructure as a side effect. Move a public type via a `[TypeForwardedTo]` forwarder; a *rename*
+   needs an `[Obsolete]` shim, not a forwarder. See `references/public-api.md`.
+
+2. **Multi-targeting and `#if`.** Code that multi-targets (`<TargetFrameworks>`) or compiles conditionally
+   (`#if NET8_0_OR_GREATER`, platform/CoreCLR/Mono/NativeAOT branches) can build for the target your editor
+   shows and break one it never invoked. Inspect every branch — including inactive ones — preserve
+   intentional divergence, and re-gate each TFM/RID. See `references/multi-targeting.md`.
+
+3. **Source-generated and partial code.** A type is often `partial` across several files, and part may be
+   **generated** (source generators, Razor, `*.g.cs`). Include every partial declaration; treat generated
+   files as derived and change the generator input/template unless the repo checks the output in as source.
+   See `references/source-generation.md`.
+
+4. **InternalsVisibleTo.** `internal` is not private across a solution: test and friend assemblies bind to
+   internals (and, when strong-named, to a public key). An internal rename/move/removal can break a
+   consumer with no reference in the declaring project. See `references/internals-visible-to.md`.
+
+## Stop and escalate (do not silently proceed)
+
+- A **public/shipped API** would change and you cannot verify compatibility — no shim/forwarder path and
+  no PublicApiAnalyzers/ApiCompat/package-validation gate to catch a break.
+- The edit changes an **observable annotation** (nullability, `[DynamicallyAccessedMembers]`,
+  `[RequiresUnreferencedCode]`, generic constraints) on a public member.
+- The change can be made consistent across **some but not all** target frameworks or platforms.
+- The only way to apply it is editing **generated output** that the next build will overwrite.
+
+## Reference files
+
+Load a reference only for a surface the inspect-first step actually found:
+
+- **[references/public-api.md](references/public-api.md)** — the two API gates and why they are not
+  interchangeable, nullable/trimming/AOT as observable API, `[Obsolete]` shims and type-forwarders,
+  suppression baselines.
+- **[references/multi-targeting.md](references/multi-targeting.md)** — TFMs, `#if` and platform symbols,
+  RIDs, and how to inspect and re-gate every target.
+- **[references/source-generation.md](references/source-generation.md)** — partial types, generated output
+  vs checked-in source, editing the generator input. (See also `dotnet-msbuild/including-generated-files`.)
+- **[references/internals-visible-to.md](references/internals-visible-to.md)** — friend/test assemblies
+  and strong-name keys.

plugins/dotnet/skills/dotnet-breaking-changes/references/internals-visible-to.md

@@ -0,0 +1,42 @@
+# InternalsVisibleTo — `internal` is not private across the solution
+
+## Contents
+
+- The hazard
+- Find the friend assemblies
+- Strong-named friends
+- What to do
+
+## The hazard
+
+`internal` limits access to the declaring assembly **unless** the assembly grants friend access via
+`[assembly: InternalsVisibleTo("Other.Assembly")]`. Test projects and split implementation assemblies use
+this constantly. So an internal rename, move, signature change, or removal can break a consumer that has
+**no project reference visible from the declaring project** — the coupling is expressed in an attribute,
+not a reference graph.
+
+## Find the friend assemblies
+
+```bash
+grep -rn "InternalsVisibleTo" --include=*.cs --include=*.csproj --include=*.props .
+```
+
+`InternalsVisibleTo` can live in a `.cs` (`AssemblyInfo`/any file) **or** as an MSBuild
+`<InternalsVisibleTo>` item in a `.csproj`/`Directory.Build.props`. Enumerate every named friend, then
+search **those** assemblies for uses of the internal symbol you are changing — not just the declaring
+project.
+
+## Strong-named friends
+
+When the declaring assembly is strong-named, the `InternalsVisibleTo` string includes the friend's full
+`PublicKey=...`. Renaming or re-signing a friend assembly, or changing keys, breaks the grant. Do not alter
+the assembly name/key half of the relationship as an incidental part of another change.
+
+## What to do
+
+- Treat internal members that friends consume with the **same care as public API**: search all friend
+  assemblies for binding references before renaming/moving/removing.
+- If the change is large, let the **compiler across the whole solution** (build the friend projects too) be
+  the safety net — a missed reference becomes a build error in the friend project, not a silent break.
+- Adding a new friend (`InternalsVisibleTo`) to make a change "reachable" is itself a surface change — flag
+  it rather than doing it silently.