From 6b60fd36dd0510591eb21d298acfd7b26d69a73e Mon Sep 17 00:00:00 2001 From: Wei Hu Date: Tue, 3 Mar 2026 07:31:25 +0000 Subject: [PATCH 1/2] Enhance mpg-sdk-migration skill for autonomous build-fix operation - Rewrite Phase 8 as Autonomous Build-Fix Loop with decision trees for error classification, fix selection, and batch strategy - Add autonomous fix recipes for each error pattern (CS0234, CS0051, CS0246, CS0111, CS1729, AZC0030, AZC0032, CS0535, CS0115) - Add autonomous rename resolution strategy using old API surface - Add generator bug detection checklist and fix vs workaround decision - Add batch fix strategy to minimize regeneration cycles - Add SQL error tracking with attempt counters and escalation criteria - Update safety rules: autonomous mode permits spec and custom code changes without asking; define clear escalation boundaries - Enhance Discovery phase: snapshot old API, extract autorest renames, identify custom code folder convention - Add ApiCompat autonomous fix recipes with code examples - Consistent @@clientName namespace syntax across both files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/skills/mpg-sdk-migration/SKILL.md | 372 ++++++++++++++---- .../mpg-sdk-migration/error-reference.md | 282 +++++++++++-- 2 files changed, 561 insertions(+), 93 deletions(-) diff --git a/.github/skills/mpg-sdk-migration/SKILL.md b/.github/skills/mpg-sdk-migration/SKILL.md index b40ef1c6fd48..d1562e43c712 100644 --- a/.github/skills/mpg-sdk-migration/SKILL.md +++ b/.github/skills/mpg-sdk-migration/SKILL.md @@ -60,7 +60,10 @@ Use **explore** agents in parallel to gather information: - `tsp-location.yaml` → already migrated (may just need version bump) - `src/autorest.md` → legacy Swagger-based, needs migration 3. **Inventory existing csharp customizations in spec**: Search all `.tsp` files in the spec directory for `@clientName("...", "csharp")` and `@@clientName` decorators. Also check for `back-compatible.tsp`. These are already applied and must not be duplicated when adding renames later. -4. **Review naming conventions**: Consult the `azure-sdk-pr-review` skill for naming review rules. +4. **Snapshot old API surface**: Read `api/.net*.cs` and extract all public type names. Store in a lookup table for later rename resolution (Phase 8). +5. **Extract autorest rename mappings**: If `src/autorest.md` exists, extract all `rename-mapping` and `prepend-rp-prefix` entries. Store for comparison after generation. +6. **Identify custom code folder convention**: Check which name the package uses: `Custom/`, `Customization/`, or `Customized/`. Match this convention for all new custom code files. +7. **Review naming conventions**: Consult the `azure-sdk-pr-review` skill for naming review rules. Present a summary plan and **ask the user** to confirm before proceeding. @@ -94,8 +97,8 @@ If `src/autorest.md` exists: 3. Delete `src/autorest.md` — git history preserves the original content. 4. Do NOT create a `client.tsp` in the SDK repo. The TypeSpec source lives in the spec repo. 5. Map remaining AutoRest directives to TypeSpec customization approach: - - Model/property renames → `@@clientName(SpecTypeName, "SdkName", "csharp")` in spec repo `client.tsp` - - Accessibility overrides → `@@access(TypeName, Access.public, "csharp")` in spec repo `client.tsp` (for types generated as `internal` that need to be `public`) + - Model/property renames → `@@clientName(SpecNamespace.SpecTypeName, "SdkName", "csharp")` in spec repo `client.tsp` + - Accessibility overrides → `@@access(SpecNamespace.TypeName, Access.public, "csharp")` in spec repo `client.tsp` (for types generated as `internal` that need to be `public`) - Suppressions → `#suppress` decorators in spec `.tsp` files - Format overrides → TypeSpec `@format` / `@encode` decorators @@ -233,91 +236,293 @@ extends: - Migrated from Swagger to TypeSpec-based generation ``` -## Phase 8 — Build Error Triage & Fix Loop +## Phase 8 — Autonomous Build-Fix Loop -After code generation, build errors (including breaking changes) surface via `dotnet build`. Each error must be triaged and fixed iteratively. +After code generation, build errors surface via `dotnet build`. This phase runs **autonomously** — Copilot triages and fixes errors without asking the user, following the decision trees below. The loop continues until `dotnet build` passes with zero errors. -### Step 1 — Collect Build Errors +### Autonomous Execution Protocol -```powershell -cd sdk\\\src -dotnet build 2>&1 | Select-String "error CS" +``` +LOOP: + 1. Run `dotnet build` → collect ALL errors + 2. IF zero errors → EXIT LOOP (proceed to Phase 9) + 3. Parse errors into structured list (error code, file, message) + 4. Group errors by root cause using Classification Algorithm + 5. For each error group, check attempt_count: + - IF attempt_count >= 5 for any error → escalate to user via ask_user + - IF error count increased from previous iteration → escalate to user + 6. Apply fixes in batch using Priority Order and Decision Tree + 7. Increment attempt_count for each error that was targeted by a fix + 8. IF any fix required spec change → regenerate with LocalSpecRepo + 9. IF any fix required generator change → regenerate with RegenSdkLocal.ps1 + 10. GOTO 1 ``` -Parse the output into a structured list of errors (file, code, message). Use a SQL table to track them: +**Priority order** (fix these first — they cascade into many other errors): +1. Missing/renamed types (CS0234, CS0246) — one `@@clientName` fixes 5–20 errors +2. Accessibility issues (CS0051, CS0122) — one `@@access` or `[CodeGenType]` fixes multiple +3. Naming rule violations (AZC0030, AZC0032) — one rename per type +4. Signature mismatches (CS1729, CS0029, CS1503) — often a spec template issue +5. Duplicate definitions (CS0111) — usually generator or spec dedup issue +6. Other errors — investigate individually -```sql -CREATE TABLE build_errors ( - id TEXT PRIMARY KEY, - error_code TEXT, - file TEXT, - message TEXT, - root_cause TEXT, -- 'spec' | 'generator' | 'customization' | 'unknown' - status TEXT DEFAULT 'pending' -- 'pending' | 'investigating' | 'fixed' | 'blocked' -); +### Error Classification Algorithm + +For each build error, classify it **without asking the user**: + +``` +Given: error in file F with message M + +1. IF F is under `src/Generated/`: + a. IF M mentions a type that exists in old API (`api/*.cs`) but with different name: + → ROOT CAUSE: spec (missing @@clientName) + b. IF M says type is "inaccessible due to protection level" (CS0051/CS0122): + → ROOT CAUSE: spec (missing @@access) or customization ([CodeGenType]) + c. IF M is about wrong constructor args, type mismatch in return types: + → ROOT CAUSE: spec (wrong template usage) or generator bug + d. IF M is AZC0030/AZC0032 (forbidden suffix): + → ROOT CAUSE: spec (needs @@clientName rename) + e. IF the generated code looks structurally wrong (missing serialization, + wrong inheritance, wrong type mapping) and the spec is correct: + → ROOT CAUSE: generator bug + +2. IF F is under `src/Custom/` or `src/Customization/` or `src/Customized/`: + a. IF M references a type that was renamed or no longer exists: + → ROOT CAUSE: spec (add @@clientName to preserve old name) OR + customization (update custom code to use new name) + b. IF M references a type that became internal: + → ROOT CAUSE: spec (@@access) or customization ([CodeGenType]) + +3. IF error is from ApiCompat (MembersMustExist, TypesMustExist): + → ROOT CAUSE: customization (need backward-compat shim) ``` -### Step 2 — Triage Each Error +### Autonomous Fix Decision Tree -For each build error, determine the root cause and look up the fix in [error-reference.md](https://github.com/Azure/azure-sdk-for-net/blob/main/.github/skills/mpg-sdk-migration/error-reference.md). The error reference contains: -- **Root cause triage table** — how to classify errors as spec, generator, or customization issues -- **Common build error patterns** — CS0234, CS0051, CS0246, AZC0030, etc. with specific fixes -- **ApiCompat error patterns** — MembersMustExist, TypesMustExist, etc. +For each classified error, apply the fix **without asking the user**: -### Step 3 — Fix Based on Root Cause +#### Decision: Spec Fix vs SDK Custom Code -For each error, apply the fix locally and regenerate. **The iteration loop continues until `dotnet build` passes with zero errors.** All three fix types (spec, generator, customization) can be combined in a single iteration — use `LocalSpecRepo` and `RegenSdkLocal.ps1` to avoid pushing to remote during the loop. +``` +PREFER spec-side fix (@@clientName, @@access) when: + - The fix is a simple rename or accessibility change + - Multiple errors would be resolved by one decorator + - The old name/accessibility is clearly documented in api/*.cs + +PREFER SDK custom code when: + - @@access doesn't work (nested/wrapper types — try spec first, fall back to [CodeGenType]) + - The fix requires adding backward-compat methods or properties + - The change is specific to this SDK and shouldn't affect the spec + - It's a one-off workaround for a generator limitation + +PREFER generator fix when: + - The same bug would affect ALL management SDKs (not just this one) + - The generated code is structurally wrong despite a correct spec + - CAUTION: Generator fixes require running Generate.ps1 to verify no regressions +``` -#### If TypeSpec issue (including naming/accessibility): -1. Update `client.tsp` (or other `.tsp` files) in the local spec repo. -2. Regenerate using the local spec repo: - ```powershell - dotnet build /t:GenerateCode /p:LocalSpecRepo= - ``` -3. Rebuild with `dotnet build` and verify the error is resolved. +#### Fix Recipe: Missing/Renamed Type (CS0234, CS0246) -#### If generator bug/gap: -1. Fix the generator code under `eng/packages/http-client-csharp-mgmt/`. -2. Regenerate with the local generator (and optionally local spec): - ```powershell - # Generator-only fix - pwsh eng/packages/http-client-csharp-mgmt/eng/scripts/RegenSdkLocal.ps1 -Services - # Generator + spec fixes together - pwsh eng/packages/http-client-csharp-mgmt/eng/scripts/RegenSdkLocal.ps1 -Services -LocalSpecRepoPath - ``` -3. Rebuild and confirm the error is resolved. -4. Create a feature branch for the generator fix and open a **draft PR** for it. -5. Continue migration work on top of that branch. +``` +1. Find the missing type name from the error message +2. Search old API surface (`api/.net*.cs`) for the old type name +3. Search new generated code (`src/Generated/`) for similar type names +4. IF a match is found with different name: + a. Find the spec namespace (e.g., Microsoft.Chaos) from the spec's main.tsp + b. Add to client.tsp in spec repo: + @@clientName(SpecNamespace.NewSpecName, "OldSdkName", "csharp"); + c. Regenerate with LocalSpecRepo +5. IF custom code references the old name and no spec rename makes sense: + a. Update custom code to use the new generated name +6. Rebuild and verify +``` -#### If customization needed: -1. Add a partial class under `src/Customization/` to address the issue. -2. Rebuild and confirm (no regeneration needed). +#### Fix Recipe: Inaccessible Type (CS0051, CS0122) -### Step 4 — Iterate Until Build Succeeds +``` +1. Find the type name from the error message +2. Check if the type exists in Generated/ as internal +3. TRY spec-side fix first: + a. Add to client.tsp: + @@access(SpecTypeName, Access.public, "csharp"); + b. Regenerate and rebuild +4. IF @@access didn't work (type is still internal after regeneration): + a. Use SDK custom code with [CodeGenType]: + [CodeGenType("OriginalSpecTypeName")] + public partial class TypeName { } + b. Place the file in the custom code folder matching the package's convention + (Custom/, Customization/, or Customized/) + c. Rebuild (no regeneration needed) +``` + +#### Fix Recipe: Forbidden Name Suffix (AZC0030, AZC0032) -**Exit condition: `dotnet build` passes with zero errors.** +``` +1. Identify the type with forbidden suffix (Request, Response, Parameters, Data) +2. Check old API surface for what the type was previously named +3. IF old name exists → rename to old name via @@clientName +4. IF no old name (new type): + a. For "Request" suffix → rename to "Content" + b. For "Response" suffix → rename to "Result" + c. For "Parameters" suffix → rename to "Content" + d. For "Data" suffix (not ResourceData) → rename to "Info" +5. Add @@clientName(SpecNamespace.SpecName, "NewName", "csharp") in client.tsp +6. Regenerate and rebuild +``` + +#### Fix Recipe: Signature Mismatch (CS1729, CS0029, CS1503) + +``` +1. Check if error is in a resource collection or resource class +2. IF error involves wrong REST client or type mismatch in collections: + a. Check if a sub-resource operation uses Read<> template in spec + b. Fix: Change Read<> to ActionSync<> with @get decorator in spec + c. Regenerate and rebuild +3. IF error involves constructor parameter mismatch: + a. Check if the generated constructor signature changed + b. IF custom code calls the old constructor: + → Update custom code to match new signature + c. IF generated code has wrong constructor (generator issue): + → Check if spec template is correct first, then consider generator fix +``` + +#### Fix Recipe: Generator Bug + +``` +1. CONFIRM it's a generator bug: + a. Verify the spec is correct (tsp compiles, models look right) + b. Verify the generated code is wrong (wrong structure, not just naming) + c. Check if the same issue exists in other generated SDKs +2. DECIDE: fix generator vs workaround + a. IF the bug is simple and isolated → fix the generator + - Edit code under eng/packages/http-client-csharp-mgmt/ + - Regenerate with RegenSdkLocal.ps1 + - Run eng/packages/http-client-csharp-mgmt/eng/scripts/Generate.ps1 to verify + - Rebuild and confirm + b. IF the bug is complex or risky → workaround with custom code + - Add [CodeGenSuppress] to hide the broken generated code + - Provide correct implementation in custom partial class + - Document the workaround and file an issue for the generator bug +3. PREFER workaround for one-off issues during migration + PREFER generator fix for systematic issues affecting multiple SDKs +``` -After fixing one error, rebuild and update the error list. Each fix may resolve multiple cascading errors or reveal new ones. Repeat until the build is clean. Update the SQL tracking table as you go: +### Autonomous Rename Resolution Strategy +When migrating from autorest, many types get renamed. Resolve renames autonomously: + +``` +1. EXTRACT old names: + a. Read api/.net*.cs for all public type names + b. Read src/autorest.md rename-mapping entries (before deleting it) + c. Store both in a lookup table + +2. AFTER code generation, COMPARE: + a. Get all new public type names from src/Generated/ + b. For each type referenced in custom code or old API surface: + - IF type exists with same name → no action needed + - IF type exists with different name → candidate for @@clientName + - IF type no longer exists → check if flattened/merged/removed + +3. FILTER OUT automatic renames (do NOT add @@clientName for these): + The mgmt emitter handles these automatically: + - RP prefix prepending (from prepend-rp-prefix in autorest.md) + - Is* boolean property naming + - Acronym casing (Ip→IP, Url→URI, etc.) + - Standard format rules + +4. For remaining mismatches, add @@clientName decorators to client.tsp + +5. For types referenced in custom code, DECIDE: + a. IF preserving old name is important (backward compat) → @@clientName + b. IF custom code can easily be updated → update custom code + c. PREFER @@clientName to minimize custom code changes +``` + +### Batch Fix Strategy + +When there are many errors (20+), fix them in batches to avoid unnecessary regeneration cycles: + +``` +1. Collect ALL errors from dotnet build +2. Group by root cause type +3. For spec fixes (@@clientName, @@access): + a. Identify ALL needed decorators from the full error list + b. Add them ALL to client.tsp in one batch + c. Run `npx tsp format "**/*.tsp"` in the spec directory + d. Regenerate ONCE with LocalSpecRepo + e. Rebuild to see remaining errors +4. For custom code fixes: + a. These don't need regeneration — fix as many as possible in one pass + b. Rebuild after all custom code changes +5. For generator fixes: + a. Fix one at a time — each may have cascading effects + b. Regenerate and rebuild after each fix +``` + +### SQL Error Tracking + +Track errors and fixes for observability: + +```sql +CREATE TABLE build_errors ( + id TEXT PRIMARY KEY, + error_code TEXT, + file TEXT, + message TEXT, + root_cause TEXT, -- 'spec' | 'generator' | 'customization' | 'unknown' + fix_type TEXT, -- 'clientName' | 'access' | 'codeGenType' | 'codeGenSuppress' | 'custom_code' | 'generator_fix' | 'spec_template' + fix_detail TEXT, -- description of what was changed + attempt_count INTEGER DEFAULT 0, -- incremented each time a fix is attempted + last_fix_attempted TEXT, -- description of the last fix tried + status TEXT DEFAULT 'pending' -- 'pending' | 'investigating' | 'fixed' | 'blocked' | 'cascaded' +); +``` + +Update as you work: ```sql -UPDATE build_errors SET status = 'fixed', root_cause = 'generator' WHERE id = 'err-1'; +-- After fixing a batch of renames +UPDATE build_errors SET status = 'fixed', root_cause = 'spec', fix_type = 'clientName', + fix_detail = 'Added @@clientName(NewName, "OldName", "csharp") in client.tsp' +WHERE error_code = 'CS0234' AND message LIKE '%OldTypeName%'; + +-- Errors resolved by cascade (fixed by another fix) +UPDATE build_errors SET status = 'cascaded' WHERE status = 'pending' AND id IN (...); ``` -**After the loop completes (build succeeds):** -Proceed to **Phase 9 — Create Pull Requests** to push changes and create separate PRs. +### Escalation Criteria -### Sub-Agent Strategy for Error Triage +The autonomous loop proceeds **without asking the user** except when: -This loop is best handled **sequentially** with a mix of agent types: +1. **Ambiguous rename** — Two equally valid old names exist and the correct choice is unclear +2. **Breaking API change with no backward-compat option** — A public type/method must be removed +3. **Generator fix affects other SDKs** — The fix would change behavior for all management SDKs +4. **Spec correctness concern** — The spec itself appears to have a bug (missing model, wrong type) beyond naming +5. **5 consecutive failed attempts** for the same error — something unexpected is happening +6. **Error count increases after a fix** — The fix made things worse -1. **task agent** — Run `dotnet build`, collect errors, populate the SQL table. -2. **For each error** (sequential — each fix may change subsequent errors): - - **explore agent** — Investigate the error: read the generated file, compare with spec, determine root cause. - - **general-purpose agent** — Apply the fix (spec change, generator fix, or customization). Include full context: error message, file path, root cause, and relevant spec/generated code. - - **task agent** — Regenerate (if spec/generator fix) and rebuild to verify. -3. **task agent** — Final full build + test to confirm all errors resolved. +For all other cases, **proceed autonomously** using the decision trees above. -> **Why sequential?** Each fix can resolve multiple errors or introduce new ones. Parallel fixing would cause conflicts and wasted work. +### Sub-Agent Strategy + +This loop is best handled with batched sequential execution: + +1. **task agent** — Run `dotnet build`, collect errors, populate the SQL table. +2. **Batch spec fixes** (most impactful first): + - **explore agent** — Analyze ALL CS0234/CS0246/CS0051/AZC0030/AZC0032 errors. Compare old API surface (`api/*.cs`) vs new generated names. Identify ALL needed `@@clientName` and `@@access` decorators. + - **general-purpose agent** — Apply ALL spec fixes to `client.tsp` in one batch. Include full context: all error messages, old API names, new generated names. + - **task agent** — Regenerate with `LocalSpecRepo` and rebuild. +3. **Batch custom code fixes**: + - **explore agent** — Analyze remaining errors in custom code, determine fixes needed. + - **general-purpose agent** — Apply custom code fixes (partial classes, `[CodeGenSuppress]`, `[CodeGenType]`, etc.). + - **task agent** — Rebuild (no regeneration needed). +4. **Generator fixes** (if any — one at a time): + - **explore agent** — Confirm generator bug, determine fix vs workaround approach. + - **general-purpose agent** — Apply generator fix or custom code workaround. + - **task agent** — Regenerate with `RegenSdkLocal.ps1` and rebuild. +5. **task agent** — Final full build to confirm all errors resolved. + +> **Why batched-sequential?** Spec fixes should be batched (one regeneration for many renames). Custom code fixes can be batched (no regeneration). Generator fixes are one-at-a-time (cascading effects). This minimizes regeneration cycles while avoiding conflicts. ## Phase 9 — Create Pull Requests @@ -409,17 +614,44 @@ After completing (or making significant progress on) a migration, review what wa ## Common Pitfalls -See [error-reference.md](https://github.com/Azure/azure-sdk-for-net/blob/main/.github/skills/mpg-sdk-migration/error-reference.md) for the full list of common pitfalls. Key ones to remember during the migration flow: +See [error-reference.md](https://github.com/Azure/azure-sdk-for-net/blob/main/.github/skills/mpg-sdk-migration/error-reference.md) for the full list of common pitfalls and autonomous fix recipes. Key ones to remember during the migration flow: - **Do NOT use `tsp-client update`** — use `dotnet build /t:GenerateCode`. - **Do NOT blindly copy all renames from `autorest.md`** — the mgmt emitter handles many automatically. -- **Build errors cascade** — fix one at a time, rebuild, and re-assess. +- **Batch spec fixes, then rebuild** — collect ALL needed `@@clientName`/`@@access` decorators before regenerating, to minimize regeneration cycles. +- **Build errors cascade** — one spec fix can resolve 5–20 errors. Always rebuild after each batch. +- **Try spec-side fix (`@@access`) before custom code (`[CodeGenType]`)** — spec-side is cleaner but doesn't work for all types. - **Finalize `tsp-location.yaml` before creating the PR** — easy to forget when using `LocalSpecRepo`. +- **Match the existing custom code folder convention** — check if the package uses `Custom/`, `Customization/`, or `Customized/`. ## Safety Rules -1. **Never modify spec files** without explicit user confirmation. -2. **Delete `autorest.md`** after extracting directives — git history preserves it. -3. **Never edit files under `Generated/`** — they are overwritten by codegen. -4. **Always ask** before making destructive changes (deleting files, removing code). -5. **Preserve git history** — prefer renames over delete+create. +### Autonomous Mode (Default) + +During the build-fix loop (Phase 8), Copilot operates autonomously. These actions are **permitted without user confirmation**: + +1. **Spec changes**: Adding `@@clientName`, `@@access`, `@@markAsPageable`, and other decorators to `client.tsp` — these are safe, reversible, and csharp-scoped. +2. **Custom code**: Adding partial classes, `[CodeGenSuppress]`, `[CodeGenType]`, `[CodeGenMember]` in the SDK custom code folder. +3. **Deleting `autorest.md`** after extracting directives — git history preserves it. +4. **Updating custom code** to reference new generated type names. +5. **Regenerating code** using `dotnet build /t:GenerateCode` or `RegenSdkLocal.ps1`. +6. **Updating CHANGELOG.md** and other metadata files. + +### Actions Requiring User Confirmation + +These actions **require explicit user approval** (use `ask_user`): + +1. **Modifying spec `.tsp` files beyond `client.tsp`** — e.g., changing `main.tsp`, model definitions, or operation signatures. These affect all languages, not just C#. +2. **Generator code changes** that affect other SDKs — run `Generate.ps1` to verify scope first. +3. **Removing public API surface** with no backward-compat option (true breaking change). +4. **Adding `ApiCompatBaseline.txt` entries** — this should almost never be done. +5. **Deleting existing custom code files** — may lose manually-written logic. + +### Hard Rules (Never Violate) + +1. **Never edit files under `Generated/`** — they are overwritten by codegen. +2. **Never hand-edit `metadata.json`** — it is auto-generated. +3. **Never use `tsp-client update`** — use `dotnet build /t:GenerateCode`. +4. **Never add entries to `ApiCompatBaseline.txt`** without explicit user approval. +5. **Never bump the major version** of a management SDK package. +6. **Preserve git history** — prefer renames over delete+create. diff --git a/.github/skills/mpg-sdk-migration/error-reference.md b/.github/skills/mpg-sdk-migration/error-reference.md index 027313c8063d..d4399683a7ba 100644 --- a/.github/skills/mpg-sdk-migration/error-reference.md +++ b/.github/skills/mpg-sdk-migration/error-reference.md @@ -1,6 +1,6 @@ # MPG Migration: Error Reference -Reference for build errors, ApiCompat errors, and common pitfalls encountered during management-plane SDK migration. +Reference for build errors, ApiCompat errors, and common pitfalls encountered during management-plane SDK migration. Each error pattern includes an **autonomous fix recipe** that Copilot can follow without user input. ## Build Error Triage @@ -15,42 +15,278 @@ For each build error, determine the root cause: ## Common Build Error Patterns -| Error Code | Typical Cause | Fix | -|-----------|--------------|-----| -| **CS0234** | Type name changed due to missing `@@clientName` rename | Add `@@clientName(SpecType, "OldSdkName", "csharp")` in `client.tsp`, regenerate | -| **CS0051** | Type became `internal` but Custom code references it publicly | First try `@@access(SpecType, Access.public, "csharp")` in `client.tsp`. If that doesn't work (common for nested/wrapper types), use `[CodeGenType("OriginalSpecName")]` in Custom code to override accessibility | -| **CS0246** | Type removed or restructured | Check if type was flattened, merged, or renamed. May need Custom code update | -| **CS0111** | Duplicate method/type definitions | For extension resources with parameterized scopes, check for duplicate resource entries. May need generator dedup fix | -| **CS1729/CS0029/CS1503** | Wrong REST client or type mismatch in collections | Sub-resource ops using `Read<>` template cause lifecycle misclassification. Fix by changing to `ActionSync<>` in spec (see the `mitigate-breaking-changes` skill, Extension Resources section) | -| **AZC0030** | Model name has forbidden suffix (`Request`, `Response`, `Parameters`) | Add `@@clientName` rename. Check old autorest SDK API surface for the **original name** to maintain backward compatibility, rather than inventing a new name | -| **AZC0032** | Model name has forbidden suffix (`Data`) not inheriting `ResourceData` | Add `@@clientName` rename to remove or replace the suffix | +### CS0234 — Type or namespace not found (renamed type) + +**Detection**: `error CS0234: The type or namespace name 'XxxType' does not exist in the namespace` + +**Autonomous Fix**: +1. Extract the missing type name from the error message (e.g., `FooBarModel`) +2. Search old API surface: `grep -r "FooBarModel" api/` +3. Search new generated code for similar names: `ls src/Generated/Models/ | grep -i foobar` +4. IF a match with different name exists (e.g., `FooBarModelInfo`): + - Determine the spec name by reading the generated file's `[CodeGenSerialization]` or class comments + - Add to `client.tsp`: `@@clientName(SpecNamespace.NewSpecName, "OldSdkName", "csharp");` +5. IF the type is referenced only in custom code and renaming makes the custom code cleaner: + - Update the custom code to use the new name instead +6. Regenerate and rebuild + +**Example**: +``` +error CS0234: The type or namespace name 'ChaosTargetType' does not exist in the namespace 'Azure.ResourceManager.Chaos.Models' +``` +→ Generated code has `ChaosTargetTypeInfo`. Add: `@@clientName(Microsoft.Chaos.TargetType, "ChaosTargetType", "csharp");` + +### CS0051 / CS0122 — Inaccessible type (internal vs public) + +**Detection**: `error CS0051: Inconsistent accessibility` or `error CS0122: is inaccessible due to its protection level` + +**Autonomous Fix**: +1. Identify the internal type from the error message +2. Find its spec name by searching `src/Generated/Internal/` for the type +3. **Try spec-side fix first**: + - Add to `client.tsp`: `@@access(SpecNamespace.SpecTypeName, Access.public, "csharp");` + - Regenerate and rebuild +4. **If @@access didn't work** (common for nested/wrapper types, types inside `Internal/` folder): + - Create a custom code file: + ```csharp + // src/Customization/Models/TypeName.cs + using Microsoft.TypeSpec.Generator.Customizations; + + namespace Azure.ResourceManager..Models + { + [CodeGenType("OriginalSpecTypeName")] + public partial class TypeName + { + } + } + ``` + - Rebuild (no regeneration needed) + +**How to find OriginalSpecTypeName**: Open the generated internal file under `src/Generated/Internal/` — the class name or `[CodeGenSerialization]` attribute reveals the original spec name. + +### CS0246 — Type or namespace could not be found + +**Detection**: `error CS0246: The type or namespace name 'Xxx' could not be found` + +**Autonomous Fix**: +1. IF the type was in a different namespace that's no longer imported: + - Add the correct `using` statement to the custom code file +2. IF the type was removed or restructured in the new spec: + - Check if the type was flattened into a parent model + - Check if it was merged with another type + - Check if it was renamed (treat as CS0234) +3. IF the type is defined in custom code that references old generated types: + - Update custom code references to use new type names +4. Regenerate if spec was changed, rebuild + +### CS0111 — Duplicate method or type definition + +**Detection**: `error CS0111: Type 'X' already defines a member called 'Y'` + +**Autonomous Fix**: +1. Check if both `Generated/` and `Custom/` define the same member +2. IF custom code intentionally overrides a generated member: + - Add `[CodeGenSuppress("MemberName", typeof(ParamType1), ...)]` to the custom partial class + - Rebuild +3. IF duplicate arises from extension resource with multiple scope entries: + - This is a known generator issue with `MockableArmClient` — check for duplicate `GetXxxResource()` methods + - Add `[CodeGenSuppress]` for the duplicate in custom code +4. Rebuild + +### CS1729 / CS0029 / CS1503 — Constructor or type mismatch + +**Detection**: `error CS1729: 'X' does not contain a constructor`, `error CS0029: Cannot implicitly convert`, `error CS1503: cannot convert from 'X' to 'Y'` + +**Autonomous Fix**: +1. IF error is in a resource collection class referencing wrong REST client: + - Check if a sub-resource operation uses `Read<>` template in the spec + - Fix: Change `Read<>` to `ActionSync<>` with `@get` in the spec + - Regenerate and rebuild +2. IF error is in custom code calling an old constructor: + - Check the new generated constructor signature + - Update custom code to match the new signature + - Rebuild +3. IF error involves collection type mismatch (e.g., `Pageable` vs `Response`): + - Check if `@@markAsPageable` is needed (see AZC pageable section) + - Or add custom code with `[CodeGenSuppress]` + wrapper method + +### AZC0030 — Forbidden model suffix + +**Detection**: `error AZC0030: The model name 'XxxRequest' ends with a forbidden suffix` + +**Autonomous Fix**: +1. Parse the forbidden type name and suffix from the error +2. Look up what the type was called in the old API: `grep "class Xxx" api/*.cs` +3. IF old name exists → use old name via `@@clientName` +4. IF new type (no old name): + - `*Request` → rename to `*Content` + - `*Response` → rename to `*Result` + - `*Parameters` → rename to `*Content` +5. Add to `client.tsp`: `@@clientName(SpecNamespace.SpecTypeName, "NewName", "csharp");` +6. Regenerate and rebuild + +### AZC0032 — Forbidden 'Data' suffix + +**Detection**: `error AZC0032: The model name 'XxxData' ends with 'Data' but does not inherit from ResourceData` + +**Autonomous Fix**: +1. Check old API for the previous name +2. IF old name exists → use old name via `@@clientName` +3. IF new type → rename to `*Info` or another appropriate suffix +4. Add `@@clientName` to `client.tsp`, regenerate and rebuild + +### CS0535 — Interface member not implemented + +**Detection**: `error CS0535: 'X' does not implement interface member 'Y'` + +**Autonomous Fix**: +1. IF the interface is `IJsonModel` or `IPersistableModel`: + - This usually means a custom type needs serialization + - Check if the type used to be generated but now needs custom implementation + - Add the missing interface methods in the custom partial class +2. IF the interface is from a base class change: + - Update custom code to implement the required members +3. Rebuild + +### CS0115 — No suitable method to override + +**Detection**: `error CS0115: no suitable method found to override` + +**Autonomous Fix**: +1. The custom code overrides a method that no longer exists in the generated base class +2. Check if the method was renamed or removed in the new generated code +3. IF renamed: Update the custom code to override the new method name +4. IF removed: Remove the override from custom code, or add `[CodeGenSuppress]` for the old method and implement fresh +5. Rebuild ## ApiCompat Error Patterns ApiCompat compares the new generated API against the existing API surface files (`api/*.cs`). ApiCompat errors surface via `dotnet pack` (not `dotnet build`). **Do NOT use `ApiCompatBaseline.txt` to bypass breaking changes** — mitigate them with custom code instead. -| ApiCompat Error | Cause | Fix | -|----------------|-------|-----| -| **MembersMustExist** (method with different return type) | Generated method has different return type than old API (e.g., `Response` vs `Pageable`) | Use `[CodeGenSuppress("MethodName", typeof(ParamType))]` on the partial class to suppress the generated method, then provide a custom implementation with the old return type | -| **MembersMustExist** (missing extension method) | Old API had `GetXxx(ArmClient, ResourceIdentifier scope, string name)` but new only has `GetXxxResource(ArmClient, ResourceIdentifier id)` | Add custom extension methods that delegate to collection Get | -| **TypesMustExist** | Old API had a type that no longer exists (e.g., base class removed) | Create the type in Custom code with matching properties and `IJsonModel<>`/`IPersistableModel<>` serialization | -| **MembersMustExist** (property setter missing) | Generated property is get-only but old API had setter | Use `[CodeGenSuppress("PropertyName")]` and re-declare with `{ get; set; }` in custom partial class | +### MembersMustExist — Method with different return type + +**Detection**: `MembersMustExist : Member 'X.Method(...)' does not exist` + +**Autonomous Fix**: +1. Compare the old method signature (from ApiCompat error) with the new generated method +2. IF return type changed (e.g., `Response` → `Pageable`): + - Add `[CodeGenSuppress("MethodName", typeof(ParamType1), typeof(ParamType2))]` on the partial class + - Provide a custom implementation with the old return type that delegates to the new method +3. IF parameter types changed: + - Add backward-compat overload in custom code with old signature + - Delegate to the new generated method + +**Example**: +```csharp +// src/Customization/MyResource.cs +public partial class MyResource +{ + [CodeGenSuppress("GetReport", typeof(CancellationToken))] + public virtual Response GetReport(CancellationToken cancellationToken = default) + { + // Delegate to new generated method with adapted return type + } +} +``` + +### MembersMustExist — Missing extension method + +**Detection**: `MembersMustExist : Member 'Extensions.GetXxx(ArmClient, ResourceIdentifier, ...)'` + +**Autonomous Fix**: +1. The old API had `GetXxx(ArmClient, ResourceIdentifier scope, string name)` convenience methods +2. New generator may only produce `GetXxxResource(ArmClient, ResourceIdentifier id)` +3. Add custom extension methods that delegate to the collection's Get: +```csharp +// src/Customization/Extensions.cs +public static partial class MyExtensions +{ + public static Response GetXxx(this ArmClient client, ResourceIdentifier scope, string name, CancellationToken cancellationToken = default) + { + return client.GetMyResources(scope).Get(name, cancellationToken); + } +} +``` + +### MembersMustExist — Missing property setter + +**Detection**: `MembersMustExist : Member 'X.Property.set'` + +**Autonomous Fix**: +1. The generated property is get-only but old API had a setter +2. Use `[CodeGenSuppress("PropertyName")]` on the partial class +3. Re-declare the property with `{ get; set; }` in custom code +4. Rebuild + +### TypesMustExist — Missing type + +**Detection**: `TypesMustExist : Type 'Namespace.TypeName' does not exist` + +**Autonomous Fix**: +1. Determine why the type no longer exists: + - **Renamed**: Add `@@clientName` in `client.tsp` to restore old name + - **Removed from spec**: Create the type in custom code with matching public API + - **Merged into another type**: Add a type alias or backward-compat wrapper +2. IF the type needs serialization (`IJsonModel`, `IPersistableModel`): + - Create a minimal implementation that delegates to the replacement type +3. Rebuild ### Handling ApiCompat Errors in the Migration Flow For each ApiCompat error: -1. **Missing member/type**: The old API had a public member that no longer exists. Determine why: - - **Renamed**: Add `@@clientName` in `client.tsp` to restore the old name, or add a backward-compat wrapper in Custom code. - - **Removed from spec**: If the member was removed in a newer API version, it may be acceptable. Document in CHANGELOG. - - **Changed signature**: Add a Custom code overload with the old signature that delegates to the new one. - - **Changed accessibility**: Use `@@access` or `[CodeGenType]` to restore public visibility. -2. After fixing all breaking changes, re-export the API surface: +1. **Run `dotnet pack --no-restore`** to surface all ApiCompat errors at once +2. **Collect all errors** into the build_errors SQL table with root_cause='customization' +3. **For each error**, apply the autonomous fix recipe above +4. After fixing all breaking changes, re-export the API surface: ```powershell pwsh eng/scripts/Export-API.ps1 ``` Where `` is the service folder name under `sdk/` (e.g., `guestconfiguration`, NOT the full package name). -3. Verify the full build passes: `dotnet build`. +5. Verify the full build passes: `dotnet build` + +## Generator Bug Detection + +When build errors don't match any of the patterns above, the issue may be in the generator. Use this checklist to confirm: + +### Is it a generator bug? + +``` +1. The spec is correct: + - `npx tsp compile .` succeeds in the spec directory + - The model/operation looks correct in the spec + - Other language generators produce correct output (if known) + +2. The generated code is structurally wrong: + - Missing serialization methods + - Wrong inheritance chain + - Incorrect type mapping (e.g., string instead of enum) + - Missing or extra properties + - Wrong REST operation mapping + +3. The issue is NOT fixable with spec decorators: + - @@clientName, @@access, @@markAsPageable won't help + - The problem is in HOW the code is generated, not WHAT is generated +``` + +### Generator fix vs workaround decision + +``` +FIX THE GENERATOR when: + - The bug is reproducible across multiple SDKs + - The fix is straightforward (< 50 lines of generator code) + - Not fixing would require complex workarounds in every affected SDK + +WORKAROUND WITH CUSTOM CODE when: + - The bug only affects this one SDK + - The generator fix would be complex or risky + - A simple [CodeGenSuppress] + custom implementation resolves it + - Time pressure — workaround is faster than understanding the generator + +After choosing workaround: + - Document the issue in a code comment + - Consider filing a GitHub issue for the generator bug +``` ## Common Pitfalls From 685a6bb8f6a3995c04f066c3fedc67273071caf9 Mon Sep 17 00:00:00 2001 From: Wei Hu Date: Tue, 3 Mar 2026 08:00:49 +0000 Subject: [PATCH 2/2] Add custom code cleanup step after generator fixes in build-fix loop After a generator fix is applied and code is regenerated with RegenSdkLocal.ps1, any custom code workarounds previously added for the same issue must be cleaned up. Stale workarounds cause CS0111 (duplicate definition) or silently override correct generated code. Updated in SKILL.md: - Fix Recipe: Generator Bug - explicit cleanup step with instructions - Sub-Agent Strategy - added cleanup agent step after generator regen Updated in error-reference.md: - Added pitfall #17 about cleaning up stale custom workarounds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/skills/mpg-sdk-migration/SKILL.md | 14 ++++++++++++-- .../skills/mpg-sdk-migration/error-reference.md | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/skills/mpg-sdk-migration/SKILL.md b/.github/skills/mpg-sdk-migration/SKILL.md index d1562e43c712..b705265bba66 100644 --- a/.github/skills/mpg-sdk-migration/SKILL.md +++ b/.github/skills/mpg-sdk-migration/SKILL.md @@ -396,8 +396,16 @@ PREFER generator fix when: 2. DECIDE: fix generator vs workaround a. IF the bug is simple and isolated → fix the generator - Edit code under eng/packages/http-client-csharp-mgmt/ - - Regenerate with RegenSdkLocal.ps1 - - Run eng/packages/http-client-csharp-mgmt/eng/scripts/Generate.ps1 to verify + - Regenerate with RegenSdkLocal.ps1 to verify the fix: + pwsh eng/packages/http-client-csharp-mgmt/eng/scripts/RegenSdkLocal.ps1 -Services + (add -LocalSpecRepoPath if spec was also changed) + - CLEAN UP: After regeneration, check if any custom code workarounds + (e.g., [CodeGenSuppress] + manual implementations) were added earlier + for the same issue. If the generator fix makes them redundant, DELETE + those custom code files/members. Stale workarounds cause CS0111 + (duplicate definition) or incorrect behavior. + - Run eng/packages/http-client-csharp-mgmt/eng/scripts/Generate.ps1 + to verify the generator fix doesn't break other SDKs - Rebuild and confirm b. IF the bug is complex or risky → workaround with custom code - Add [CodeGenSuppress] to hide the broken generated code @@ -520,6 +528,8 @@ This loop is best handled with batched sequential execution: - **explore agent** — Confirm generator bug, determine fix vs workaround approach. - **general-purpose agent** — Apply generator fix or custom code workaround. - **task agent** — Regenerate with `RegenSdkLocal.ps1` and rebuild. + - **general-purpose agent** — After generator fix, check for stale custom code workarounds that were added for the same issue earlier. Remove any `[CodeGenSuppress]` + manual implementations that are now redundant because the generator produces correct output. + - **task agent** — Rebuild again after cleanup to confirm no regressions. 5. **task agent** — Final full build to confirm all errors resolved. > **Why batched-sequential?** Spec fixes should be batched (one regeneration for many renames). Custom code fixes can be batched (no regeneration). Generator fixes are one-at-a-time (cascading effects). This minimizes regeneration cycles while avoiding conflicts. diff --git a/.github/skills/mpg-sdk-migration/error-reference.md b/.github/skills/mpg-sdk-migration/error-reference.md index d4399683a7ba..1765e8fa1832 100644 --- a/.github/skills/mpg-sdk-migration/error-reference.md +++ b/.github/skills/mpg-sdk-migration/error-reference.md @@ -336,3 +336,5 @@ After choosing workaround: This is the single most reliable way to verify your changes will pass the "Build Analyze PRBatch" CI check. 16. **Finalize `tsp-location.yaml` before creating the PR.** When using `LocalSpecRepo` for fast iteration, it's easy to forget to update `tsp-location.yaml` with the final commit SHA. Before creating the PR: (a) commit and push all spec changes, (b) get the final commit SHA, (c) update `tsp-location.yaml` with that SHA, and (d) do one final `dotnet build /t:GenerateCode` **without** `LocalSpecRepo` to verify CI-reproducible generation. + +17. **Clean up stale custom workarounds after generator fixes.** When a generator bug is fixed and the SDK is regenerated with `RegenSdkLocal.ps1`, any custom code workarounds that were added earlier for the same issue (e.g., `[CodeGenSuppress]` + manual implementations) become stale. These must be removed after regeneration — otherwise they cause `CS0111` (duplicate definition) errors or silently override the now-correct generated code. After each generator fix + regeneration cycle, search custom code for `[CodeGenSuppress]` attributes referencing the fixed member and remove them along with the manual implementation.