Azure .NET Mgmt SDK Migration PR Review

Review Azure SDK for .NET management-library migration PRs from Swagger/AutoRest to TypeSpec. This skill extends azure-sdk-mgmt-pr-review; run its Phases 1-3 first, then run these migration phases.

Use this skill for migration PRs indicated by titles like Migrate to TypeSpec / MPG migration, adding tsp-location.yaml, deleting src/autorest.md, adding TypeSpec metadata.json, or broadly regenerating src/Generated/. Do not use it for normal TypeSpec-based management feature PRs unless they change migration shape or mitigation code.

Phases:

1. Versioning Review - base skill.
2. API Review - base skill.
3. Breaking Change Detection - base skill.
4. Migration Customization Review.
5. TypeSpec Decorator Preference Review.
6. Migration New API Triage.

Migration notes for base phases:

• Phase 2: migration often introduces generic C# names such as Scope, GroupScope, Sensitivity, ManagedRuleSetException; flag contextual naming issues and require @@clientName(..., "csharp") when defined in TypeSpec.
• Phase 3: migration breaking changes are expected but still must be mitigated. ApiCompat baseline entries remain forbidden except targeted WirePathAttribute removal entries in the centralized baseline file.

Phase 4: Migration Customization Review

Review added/modified files under src/Custom*/, src/Customization*/, or src/Customized*/. Comment on source/customization/TypeSpec lines in the PR diff; never target api/*.cs inline. Treat issues as blocking when they risk incorrect API shape, broken compatibility, generated-code drift, or unmaintainable migration debt; minor cleanup can be a suggestion.

Rules:

• 4.1 Justification comments: each customization file or significant block must explain why it exists: breaking-change mitigation, generator bug with issue link, naming fix, backward compatibility, etc.
• 4.2 [CodeGenMember] vs [CodeGenSuppress]: after confirming the member cannot be renamed in TypeSpec, use [CodeGenMember("GeneratedName")] for same-data renames so generator metadata, serialization, and samples remain linked. Use [CodeGenSuppress("MemberName", ...)] only when removing/suppressing a generated member or rewriting different behavior.
• 4.3 [CodeGenType] is not a spec-defined rename mechanism - blocking: do not accept [CodeGenType("SpecName")] custom partials that only rename a model, enum, union, property container, or other API that is directly defined in the service TypeSpec. Require @@clientName(TypeSpecTarget, "CSharpName", "csharp") in client.tsp and regeneration instead. [CodeGenType] is allowed only when the generated C# artifact is synthesized/not directly targetable in TypeSpec, or when it is part of a necessary GA compatibility shim that cannot replace the generated name.
• 4.4 EditorBrowsable(Never) misuse: only hide backward-compat members that have replacements. Never hide the only public constructor, method, or property for its purpose.
• 4.5 Rename in TypeSpec, don't hide-and-replace - blocking: when a TypeSpec-defined API has the wrong C# name, require a scoped @@clientName(..., "csharp") customization in the spec and regeneration. Do not suggest or accept [CodeGenType], [CodeGenMember], [CodeGenSuppress], wrapper members, or forwarding methods merely to rename that API. If the artifact is synthesized by the C# generator and is not directly targetable in TypeSpec, use the smallest SDK customization. A custom old-name method that only forwards to a new generated method usually means the generated method should be renamed back.
• 4.6 ModelFactory anti-patterns: do not suppress generated ModelFactory methods without strong justification. Backward-compat overloads should delegate to generated public ModelFactory overloads and translate renamed parameters/types as needed; do not call internal constructors or private Core helpers. Add only overloads that existed in the previous stable API.
• 4.7 Constructor suppression: if a generated constructor has the wrong signature, first fix root cause in client.tsp: missing flattened properties -> @@flattenProperty; wrong property type -> @@alternateType; missing Properties envelope members -> flatten the envelope. Review every constructor [CodeGenSuppress] for a possible decorator fix.
• 4.8 Using cleanup: flag customization-file diffs that only add/remove unnecessary using statements.
• 4.9 Obsolete compatibility members: determine why [Obsolete] exists. A backward-compat shim preserving shipped API should remain functional when it can safely delegate to the replacement; do not require NotSupportedException for safe shims. Obsolete properties must not throw from getters just because the old value is no longer populated: blind property traversal and reflection-based tooling can hit them unexpectedly. Prefer a non-throwing auto-property that preserves the previous accessor shape, for example do not add a setter when the shipped property was get-only, with an obsolete message that points to the replacement or explains that the property no longer works. Require NotSupportedException only when old behavior cannot be implemented safely/accurately or is intentionally unsupported.
• 4.10 Method input collection types: flag externally visible public API/method input parameters typed as IList<T> or IList; do not flag private/internal helper methods. MPG migration compatibility shims and generated-surface fixes should avoid requiring a mutable list implementation from callers; prefer the previous GA signature when preserving compatibility, otherwise use a more appropriate input shape such as IEnumerable<T> or another established SDK pattern for that operation.
• 4.11 No manual edits to generated files - blocking: src/Generated/ must be generator output only. A large regenerated diff is expected and not evidence of hand editing. Flag clear hand edits such as isolated generated-style mismatches, custom logic, ad hoc #pragma suppressions, type changes, or CI Verify Generated Code/code-check drift. Fix through custom partials ([CodeGenSuppress]), TypeSpec decorators (@@clientName, @@alternateType, @@access), or generator bug fixes. After adding custom files with generator attributes, regenerate so suppressions are honored.

Compact examples:

• Bad TypeSpec-defined type rename: [CodeGenType("CommunityEndpointProtocol")] public partial struct VirtualEnclaveCommunityEndpointProtocol.
• Good TypeSpec-defined type rename: @@clientName(CommunityEndpointProtocol, "VirtualEnclaveCommunityEndpointProtocol", "csharp"); in client.tsp, followed by regeneration.
• Bad rename: [CodeGenSuppress("Tls1_0")] plus custom Tls10 member.
• Good synthesized-member rename when TypeSpec cannot target it: [CodeGenMember("Tls1_0")] public static RedisTlsVersion Tls10 { get; } = ...;.
• Bad ModelFactory compatibility: private helper calling internal generated constructors.
• Good ModelFactory compatibility: compatibility overload delegates to ArmRedisModelFactory.RedisData(...).
• Bad generated edit: changing src/Generated/Models/SomeModel.cs by hand.
• Good generated fix: suppress in src/Custom/SomeModel.cs, provide custom member, then regenerate.

Phase 5: TypeSpec Decorator Preference Review

Prefer TypeSpec decorators over SDK custom code when they can express the fix. Decorators are easier to maintain, keep migration behavior centralized in TypeSpec/customization inputs, and reduce custom code.

Always include the "csharp" scope parameter for decorators in azure-rest-api-specs; unscoped decorators affect other language emitters.

Common replacements:

• [CodeGenType], [CodeGenMember], suppression, or wrapper code used only to rename a TypeSpec-defined API -> @@clientName(TypeSpecTarget, "CSharpName", "csharp").
• Manual wrapper properties for an unflattened Properties envelope -> @@flattenProperty(Model.properties, "csharp").
• Manual property type conversion (string -> ResourceIdentifier, AzureLocation, etc.) plus custom serialization -> @@alternateType(Model.prop, TargetType, "csharp").
• Custom pageable wrappers for old Pageable<T> / AsyncPageable<T> shape -> @@markAsPageable(Interface.operation, "csharp"), with #suppress "@azure-tools/typespec-azure-core/no-legacy-usage" "migration" when needed.
• Public accessibility for an internal generated type -> try @@access(Type, Access.public, "csharp") before [CodeGenType]; fall back only when @@access is ineffective, such as nested/wrapper generated types.

When suggesting a decorator, include a scoped example and target the relevant client.tsp/TypeSpec line when in the diff.

On re-review, inspect every newly added rename customization and compare it with prior review guidance. If an earlier comment requested a TypeSpec rename but the author added SDK custom code instead, keep the finding blocking; do not mark the naming issue resolved. The review summary must distinguish "API has the requested name" from "rename implemented in the required layer."

Phase 6: Migration New API Triage

Migration PRs can show many additive public APIs after regeneration. Classify each meaningful new type/member from the API diff before accepting it.

1. Compare PR API against previous GA API from ApiCompatVersion when available.
2. For each meaningful new public API, classify it:

Use operation IDs, generated XML docs, request paths, resource type/parent hierarchy, and previous GA API listings to distinguish real additions from renamed existing APIs. When a spec-level rename or decorator can produce the shipped API shape, do not keep both old and new names unless there is an approved deliberate reason. When the new API is the correct generated shape from TypeSpec and cannot be renamed in spec, keep it and preserve the old GA API shape for compatibility.

Duplicate API detection:

• Scan for duplicate model/type shapes (Foo, FooResource, FooData, FooGenerated, FooContent), duplicate extensible enums with the same wire values or case-only members, and duplicate methods/overloads for the same operation ID, route, resource type, and wire parameters.
• Compare each suspected duplicate with the previous GA API and the TypeSpec source. Post a duplicate-API alarm only when the extra public API can be removed by renaming the TypeSpec-defined element with @@clientName(..., "csharp") or another supported spec-level decorator.
• Do not alarm on duplicates that are the correct generated shape from TypeSpec but cannot be renamed in spec. In those cases, honor the generated code from the spec and keep the previous GA shape through minimal compatibility customizations so ApiCompat remains satisfied.
• Prefer spec-level renames for TypeSpec-defined duplicates. Use SDK customizations such as [CodeGenType], [CodeGenMember], or [CodeGenSuppress] only for generated C# artifacts that are not directly expressible in TypeSpec or for the GA compatibility shim.

Output Format

Follow base skill output rules: one PR review, safe-output tools in GitHub Agentic Workflow mode, no direct GitHub writes from agentic workflows, no execution of untrusted PR code, and no inline comments on api/*.cs.

Each migration inline comment should start with a rule ID such as **[4.1]**, **[4.11]**, or **[5.2]**, explain the violation, and suggest the fix. Put unattachable findings in Non-inline findings.

Review body must include:

• Phase 1-3 results from the base skill.
• Phase 4 customization result, grouped by 4.1-4.11.
• Phase 5 decorator-preference result.
• Phase 6 new API triage result, including accepted real additions, flagged rename/drift counts, and duplicate model/enum/operation counts.
• Final counts for critical issues, should-fix issues, and suggestions.