Mgmt Review Comment Resolution

Skill: mgmt-review-comment-resolution

Resolve review comments on Azure management-plane SDK PRs by making changes in the TypeSpec spec repo (client.tsp) and regenerating the SDK.

When Invoked

Trigger phrases: "resolve review comments", "fix review comments", "resolve PR comments", "rename types", "fix naming", "resolve naming comments", "fix API surface".

Overview

Management-plane SDK code is generated from TypeSpec definitions in azure-rest-api-specs. Review comments on SDK PRs often request changes to the generated API surface — renaming types, renaming properties, or changing property types to be more user-friendly. These changes are made in the spec repo's client.tsp using TypeSpec decorators (primarily @@clientName), then the SDK is regenerated.

Key Concepts

• @@clientName(TypeSpecName, "CSharpName", "csharp") — renames a TypeSpec model/union/enum/property to a different name in the generated C# SDK. This is the primary tool for resolving naming comments.
• @@clientName only works on types defined in the service's TypeSpec — you can only target models, unions, enums, and interfaces that are explicitly declared in the service's .tsp files. If a type is not defined there (e.g., it comes from ARM common types or TypeSpec templates), @@clientName cannot target it. See "Renaming via SDK Custom Code" below.
• @@clientName cannot target aliases — if a type is defined as an alias, @@clientName cannot target it. See "Handling Unsupported Cases" below.
• Only client.tsp may be changed in the spec repo — do not modify any other .tsp files or config files. If resolving a comment would require changes to other files, skip that change and inform the user with the reason after processing all other comments. Since only client.tsp is modified, swagger regeneration is never needed.

Types of Review Comments

1. Type Names Too Generic

Review comments may flag type names that are too generic and could collide with types from other SDKs or lack sufficient context.

Goal: Make the name more descriptive by adding context — this could be the RP name, the resource name, or other domain-specific context. The right prefix depends on what makes the name unambiguous and meaningful.

Examples:

How to decide the prefix: Read the review comment carefully — it may suggest a specific prefix. If not, choose the most natural context that makes the name unambiguous within the SDK's namespace. When uncertain, ask the user what prefix or name they prefer.

2. Property Renames

Properties can also be renamed using @@clientName on the model's property:

@@clientName(MyModel.oldPropertyName, "NewPropertyName", "csharp");

3. Property Type Changes

Some review comments ask for property types to be changed (e.g., string → ResourceIdentifier, string → TimeSpan). Use the @@alternateType decorator in client.tsp to change the generated C# type:

@@alternateType(MyModel.myProperty, armResourceIdentifier, "csharp");
@@alternateType(MyModel.durationProperty, duration, "csharp");

This changes the property's type in the generated C# SDK without modifying other .tsp files. If @@alternateType cannot handle the requested type change, skip the change and inform the user with the reason after processing all other comments.

Workflow

Step 1: Read and Understand Review Comments

Fetch the PR's review comments using GitHub MCP tools. For each unresolved comment:

1. Identify what change is requested (rename type, rename property, change type, etc.)
2. Find the corresponding TypeSpec type/property in the spec
3. Determine the appropriate new name or change
4. Determine if the type is defined in the service's TypeSpec: Search all .tsp files under the spec folder for a model, union, or enum declaration with the same name.
   • If defined → the rename will use @@clientName in client.tsp (Step 3)
   • If NOT defined → the rename must use SDK-side custom code (Step 3b)

If a comment is ambiguous about what the new name should be, ask the user before proceeding.

Step 2: Locate the Spec Files

1. Find tsp-location.yaml in the SDK package directory:

   sdk/<service>/Azure.ResourceManager.<Service>/tsp-location.yaml
   

2. This file contains:

   • directory: path to the TypeSpec spec (e.g., specification/managedoperations/ManagedOps.Management)
   • commit: the spec repo commit SHA
   • repo: the spec repo (e.g., Azure/azure-rest-api-specs)

3. Key spec files:

   • client.tsp — where @@clientName and @@alternateType decorators go (this is the only .tsp file you should modify)
   • Other .tsp files — read-only reference for understanding models, unions, and aliases

Step 3: Apply Changes in the Spec Repo (for types defined in TypeSpec)

This step applies only to types that are defined in the service's TypeSpec files. For types NOT defined in the service's TypeSpec, skip to Step 3b.

Locate the spec repo. It may be:

• A local clone (typically at ../azure-rest-api-specs relative to the SDK repo)
• If not available locally, clone it or ask the user for the path

If there is an existing spec PR branch, check it out. If there is no spec PR or branch yet, create a new branch from the commit referenced in tsp-location.yaml.

Add @@clientName decorators to client.tsp. Read the existing client.tsp to find the correct using statement for the service namespace (e.g., using Microsoft.ManagedOps;, using Azure.ResourceManager.Compute;, etc.), then add decorators targeting the types to rename:

@@clientName(GenericTypeName, "MoreDescriptiveTypeName", "csharp");
@@clientName(MyModel.genericProperty, "moreDescriptiveProperty", "csharp");

Make sure the using statement for the service namespace is present so that type names resolve correctly. Check the namespace declaration in main.tsp or other .tsp files to find the correct namespace.

Step 3b: Apply Changes via SDK Custom Code (for types NOT defined in TypeSpec)

Types that are not defined in the service's TypeSpec (e.g., from ARM common types or TypeSpec templates) cannot be renamed via @@clientName. Instead, create SDK-side customization files to rename them.

1. Create a customization file in the SDK package's src/Customize/Models/ directory (create the directory if it doesn't exist).
2. Define a partial class (or partial struct for value types) with the desired name, and apply the [CodeGenType("OriginalGeneratedName")] attribute to map it to the generated type.

Example: To rename OptionalPropertiesUpdateableProperties to DurableTaskPrivateEndpointConnectionPatchProperties:

// src/Customize/Models/DurableTaskPrivateEndpointConnectionPatchProperties.cs
using Azure.ResourceManager.DurableTask.Models;

namespace Azure.ResourceManager.DurableTask.Models
{
    [CodeGenType("OptionalPropertiesUpdateableProperties")]
    public partial class DurableTaskPrivateEndpointConnectionPatchProperties
    {
    }
}

After creating customization files, proceed to Step 6 (Regenerate the SDK) — Steps 4 and 5 (commit spec changes and update tsp-location.yaml) are only needed if client.tsp was also modified.

Step 4: Commit and Push Spec Changes

Commit client.tsp changes and push to the spec PR branch.

Step 5: Update SDK tsp-location.yaml

Update the commit field in tsp-location.yaml to point to the new spec commit SHA:

directory: specification/<service>/<ServiceName>.Management
commit: <new-commit-sha>
repo: Azure/azure-rest-api-specs

Step 6: Regenerate the SDK

cd sdk/<service>/Azure.ResourceManager.<Service>/src
dotnet build /t:GenerateCode

Step 7: Build and Verify

# Build the library and tests (mgmt libraries always have a solution file)
cd sdk/<service>/Azure.ResourceManager.<Service>
dotnet build

# Export API listing
cd <repo-root>
pwsh eng/scripts/Export-API.ps1 <service-directory>

If tests fail to build due to old type/property names, update the test files to use the new names.

Step 8: Verify the Changes

Check the API surface file (api/*.cs) to confirm the review comments have been addressed — old names are gone and new names are present.

Step 9: Commit, Push, and Resolve Comments

1. Commit and push the regenerated SDK to the SDK PR branch
2. Resolve the corresponding review comments on the SDK PR using GitHub GraphQL API

Handling Unsupported Cases

Some changes cannot be done purely through @@clientName or @@alternateType in client.tsp. When you encounter any of the following, skip the change and continue processing other comments. After all processable changes are done, inform the user about the skipped changes with the reason, and let them choose how to proceed:

• Aliases — @@clientName cannot target TypeSpec aliases (e.g., alias foo = "A" | "B" | string;). Explain to the user that the alias needs to be converted to a named type (e.g., union) in the spec .tsp file first.
• Changes requiring spec file modifications beyond client.tsp — any change that cannot be expressed via client.tsp decorators or SDK custom code alone (e.g., structural changes, flattening/unflattening models, adding/removing properties, or changing inheritance).
• Ambiguous naming — if a review comment says "rename this" but doesn't specify the new name, ask the user what name they prefer.

Common Pitfalls

1. Missing using statement in client.tsp — the type names won't resolve and @@clientName will silently fail. Check the namespace declaration in the spec's .tsp files to find the correct namespace.
2. Modifying files other than client.tsp — only client.tsp should be modified in the spec repo. If other file changes are needed, skip and inform the user.
3. Stale commit SHA — always update tsp-location.yaml to point to the pushed spec commit before regenerating.
4. Test files — after renaming types, test and sample files may still reference old names and need manual updates.
5. Not building tests — always build the solution (which includes tests), not just the library, to catch stale references.