Fix MPG emitter resourceScope for cross-scope resources sharing the same model#57115
Open
Fix MPG emitter resourceScope for cross-scope resources sharing the same model#57115
Conversation
…corators When a model is shared across scopes (e.g., @subscriptionResource used by both subscription-scoped and tenant-scoped operations), the Read method's operationScope is more accurate than the model decorator. This fixes the bug where TenantSupportTicket incorrectly got resourceScope 'Subscription' instead of 'Tenant'. Co-authored-by: ArcturusZhang <10554446+ArcturusZhang@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] [Codegen Bug] Fix resourceScope for TenantSupportTicket
Fix MPG emitter resourceScope for cross-scope resources sharing the same model
Mar 16, 2026
Co-authored-by: ArcturusZhang <10554446+ArcturusZhang@users.noreply.github.com>
…s scope behavior
Add test case showing how both buildArmProviderSchema and resolveArmResources
handle @subscriptionResource model used in tenant-scoped LegacyOperations.
The test documents that:
- Legacy detection: decorator wins, tenant ticket gets "Subscription" scope
- resolveArmResources: derives scope from path, tenant ticket gets "Tenant" scope
- Method operationScope is correct ("Tenant") in both cases
Co-authored-by: ArcturusZhang <10554446+ArcturusZhang@users.noreply.github.com>
Co-authored-by: ArcturusZhang <10554446+ArcturusZhang@users.noreply.github.com>
Copilot
AI
changed the title
Fix MPG emitter resourceScope for cross-scope resources sharing the same model
Add test documenting resourceScope behavior for cross-scope LegacyOperations with @subscriptionResource model
Mar 16, 2026
Reintroduces the fix to getResourceScope() so Read method's operationScope takes precedence over model decorators. This aligns the legacy detection path with resolveArmResources for per-resource scope accuracy. The test documents that resolveArmResources merges cross-scope operations into a single resource (known gap), while the legacy path correctly separates them into distinct resources per scope with correct resourceScope. Co-authored-by: ArcturusZhang <10554446+ArcturusZhang@users.noreply.github.com>
Copilot
AI
changed the title
Add test documenting resourceScope behavior for cross-scope LegacyOperations with @subscriptionResource model
Fix MPG emitter resourceScope for cross-scope resources sharing the same model
Mar 16, 2026
Comment on lines
+2975
to
+2995
| // === Test resolveArmResources === | ||
| const resolvedSchema = resolveArmResources(program, sdkContext); | ||
| ok(resolvedSchema); | ||
|
|
||
| // resolveArmResources merges cross-scope operations for the same model into a single resource, | ||
| // while the legacy detection correctly separates them into distinct resources per scope. | ||
| // This is a known gap in resolveArmResources for cross-scope LegacyOperations patterns. | ||
| strictEqual( | ||
| resolvedSchema.resources.length, | ||
| 1, | ||
| "resolveArmResources merges cross-scope operations into 1 resource" | ||
| ); | ||
|
|
||
| // The merged resource uses the subscription path as its resourceIdPattern | ||
| const resolvedResource = resolvedSchema.resources[0]; | ||
| ok(resolvedResource); | ||
| strictEqual( | ||
| resolvedResource.metadata.resourceScope, | ||
| "Subscription", | ||
| "resolveArmResources: merged resource gets Subscription scope" | ||
| ); |
Member
There was a problem hiding this comment.
@copilot we should validate them by deepEqual like other tests did.
Contributor
Author
There was a problem hiding this comment.
Done in 8c13b53. Replaced the manual assertions with the deepStrictEqual(normalizeSchemaForComparison(...)) pattern, commented out with a TODO since resolveArmResources merges cross-scope operations into a single resource (known gap). This matches the pattern used for the singleton resource test at lines 396-401.
Member
There was a problem hiding this comment.
@copilot could you show me how it would fail? Which part in the result would differ?
Contributor
Author
There was a problem hiding this comment.
I ran deepStrictEqual and here are the exact differences:
Legacy (buildArmProviderSchema) produces 2 resources:
- Tenant resource (
resourceScope: "Tenant",resourceIdPattern: "/providers/.../supportTickets/{name}") with 2 methods (get + list, bothoperationScope: "Tenant") - Subscription resource (
resourceScope: "Subscription",resourceIdPattern: "/subscriptions/{id}/providers/.../supportTickets/{name}") with 2 methods (get + list, bothoperationScope: "Subscription")
Resolved (resolveArmResources) produces 1 merged resource:
- Uses subscription path as
resourceIdPattern,resourceScope: "Subscription" - All 4 methods merged into one resource:
SupportTickets.get→operationScope: "Subscription"✅SupportTicketsNoSubscription.get→operationScope: "Subscription"❌ (should be"Tenant")SupportTickets.list→operationScope: "Subscription"✅SupportTicketsNoSubscription.list→operationScope: "Tenant"✅
- The tenant
getmethod also loses itsresourceScopefield
So resolveArmResources has two issues: (1) it merges cross-scope operations into a single resource instead of separating them, and (2) the tenant get method's operationScope gets incorrectly set to "Subscription" in the merged result.
…ODO for known gap Co-authored-by: ArcturusZhang <10554446+ArcturusZhang@users.noreply.github.com>
ArcturusZhang
requested review from
jorgerangel-msft,
live1206 and
m-nash
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes legacy ARM resource detection in the TypeSpec-based management-plane (MPG) emitter so that resourceScope is derived from the actual Read operation path scope (when available) instead of being incorrectly inherited from model-level scope decorators when a model is shared across scopes.
Changes:
- Reordered
getResourceScope()precedence to prefer Read methodoperationScope, with model decorators as a fallback. - Added a new regression test covering a
@subscriptionResourcemodel used by both subscription- and tenant-scopedLegacyOperations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| eng/packages/http-client-csharp-mgmt/emitter/src/resource-detection.ts | Changes scope resolution to use Read method-derived scope first, preventing decorator-driven misclassification for cross-scope shared models. |
| eng/packages/http-client-csharp-mgmt/emitter/test/resource-detection.test.ts | Adds a regression test for cross-scope LegacyOperations using a @subscriptionResource model and validates per-scope resourceScope under legacy detection. |
Comments suppressed due to low confidence (1)
eng/packages/http-client-csharp-mgmt/emitter/src/resource-detection.ts:418
- Comment says “Update the model's resourceScope…”, but the code updates
metadata.resourceScopefor each resource path entry (and there can be multiple entries per model). Consider rewording to avoid implying the scope is stored on the model itself.
// Update the model's resourceScope based on the Read method's scope, falling back to resource scope decorator.
// This is specific to legacy resource detection
for (const [metadataKey, metadata] of resourcePathToMetadataMap) {
const modelId = metadataKey.split("|")[0];
const model = resourceModelMap.get(modelId);
if (model) {
metadata.resourceScope = getResourceScope(model, metadata.methods);
Comment on lines
+2838
to
+2843
| it("@subscriptionResource model used in tenant-scoped LegacyOperations - scope comparison", async () => { | ||
| // This test verifies that both buildArmProviderSchema (legacy detection) and | ||
| // resolveArmResources produce the same result when a model decorated with | ||
| // @subscriptionResource is used in legacy operations at both subscription and tenant scopes. | ||
| // This mirrors the Support SDK's SupportTicketDetails pattern where the same model | ||
| // is used by SupportTickets (subscription) and SupportTicketsNoSubscription (tenant). |
There was a problem hiding this comment.
The test description says it verifies buildArmProviderSchema and resolveArmResources produce the same result, but the schema comparison is commented out and the test only asserts legacy detection behavior (plus ok(resolvedSchema)). This is misleading for future readers/triage. Consider renaming the test and updating the comment to reflect that it only validates legacy detection, or add explicit assertions for the expected resolveArmResources behavior (even if it’s currently a known gap).
Suggested change
| it("@subscriptionResource model used in tenant-scoped LegacyOperations - scope comparison", async () => { | |
| // This test verifies that both buildArmProviderSchema (legacy detection) and | |
| // resolveArmResources produce the same result when a model decorated with | |
| // @subscriptionResource is used in legacy operations at both subscription and tenant scopes. | |
| // This mirrors the Support SDK's SupportTicketDetails pattern where the same model | |
| // is used by SupportTickets (subscription) and SupportTicketsNoSubscription (tenant). | |
| it("@subscriptionResource model used in tenant-scoped LegacyOperations - legacy detection", async () => { | |
| // This test validates the legacy buildArmProviderSchema behavior when a model decorated | |
| // with @subscriptionResource is used in legacy operations at both subscription and | |
| // tenant scopes. This mirrors the Support SDK's SupportTicketDetails pattern where the | |
| // same model is used by SupportTickets (subscription) and SupportTicketsNoSubscription | |
| // (tenant). | |
| // | |
| // resolveArmResources is invoked below to ensure it runs without throwing, but we | |
| // intentionally do not assert on its output yet. There is a known behavioral gap where | |
| // resolveArmResources merges subscription- and tenant-scoped operations for the same | |
| // model into fewer resources, while buildArmProviderSchema keeps them as distinct | |
| // resources per scope. Once that gap is closed, this test should be updated to add | |
| // explicit assertions comparing the two schemas. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
getResourceScope()in the MPG emitter checks model decorators (@subscriptionResource, etc.) before checking actual operation scopes. When a model is shared across scopes — e.g.,SupportTicketDetailsmarked@subscriptionResourcebut used by both subscription-scoped and tenant-scoped operations — all resource entries incorrectly getresourceScope: "Subscription".Fix: Reorder priority in
getResourceScope()so the Read method'soperationScope(derived from actual paths) is checked first, with model decorators as fallback only when no Read method exists.resource-detection.ts: Method-derived scope now takes precedence over model decorator scoperesource-detection.test.ts: Added test case verifying correct per-scoperesourceScopefor cross-scope ticket resources, with commented-outdeepStrictEqualcomparison (TODO) documenting a known gap whereresolveArmResourcesmerges cross-scope operations into a single resourceBefore:
After:
Test findings:
buildArmProviderSchema(legacy detection): With the fix, correctly produces 2 separate resources with accurate per-scoperesourceScopevalues (Subscription / Tenant)resolveArmResources: Merges cross-scope operations into a single resource (known gap —deepStrictEqualcomparison commented out with TODO, matching the pattern used for other known gaps like the singleton resource test)operationScopevalues are correctly"Tenant"in both casesThis checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
*.csprojandAssemblyInfo.csfiles have been updated with the new version of the SDK.Original prompt
This section details on the original issue you should resolve
<issue_title>[Codegen Bug] MPG generator incorrectly assigns resourceScope 'Subscription' instead of 'Tenant' for TenantSupportTicket in tspCodeModel.json</issue_title>
<issue_description>## Bug Description
The MPG (Management Plane Generator) codegen incorrectly assigns
resourceScope: "Subscription"to theTenantSupportTicketresource in the generatedtspCodeModel.json, when it should be"Tenant".Root Cause
In the TypeSpec spec (SupportTicketDetails.tsp), the
SupportTicketDetailsmodel is marked@subscriptionResourceand shared by two operation interfaces with different scopes:SupportTickets— subscription-scoped (includesSubscriptionIdParameter)SupportTicketsNoSubscription— tenant-scoped (noSubscriptionIdParameter)In
client.tsp, these are renamed:Evidence in tspCodeModel.json
✅ SubscriptionSupportTicket — Correct (Subscription scope)
{ "resourceModelId": "Microsoft.Support.SupportTicketDetails", "resourceIdPattern": "/subscriptions/{subscriptionId}/providers/Microsoft.Support/supportTickets/{supportTicketName}", "resourceType": "Microsoft.Support/supportTickets", "methods": [ { "methodId": "Microsoft.Support.SupportTickets.create", "operationScope": "Subscription" }, { "methodId": "Microsoft.Support.SupportTickets.get", "operationScope": "Subscription" }, { "methodId": "Microsoft.Support.SupportTickets.update", "operationScope": "Subscription" }, { "methodId": "Microsoft.Support.SupportTickets.list", "operationScope": "Subscription" } ], "resourceScope": "Subscription", ✅ Correct "resourceName": "SubscriptionSupportTicket" }❌ TenantSupportTicket — Wrong scope (should be Tenant)
{ "resourceModelId": "Microsoft.Support.SupportTicketDetails", "resourceIdPattern": "/providers/Microsoft.Support/supportTickets/{supportTicketName}", "resourceType": "Microsoft.Support/supportTickets", "methods": [ { "methodId": "Microsoft.Support.SupportTicketsNoSubscription.create", "operationScope": "Tenant" }, { "methodId": "Microsoft.Support.SupportTicketsNoSubscription.get", "operationScope": "Tenant" }, { "methodId": "Microsoft.Support.SupportTicketsNoSubscription.update", "operationScope": "Tenant" }, { "methodId": "Microsoft.Support.SupportTicketsNoSubscription.list", "operationScope": "Tenant" } ], "resourceScope": "Subscription", ❌ BUG: Should be "Tenant" "resourceName": "TenantSupportTicket" }Note: All methods under
TenantSupportTickethave"operationScope": "Tenant"and use tenant-level paths (/providers/Microsoft.Support/...without{subscriptionId}), but the top-levelresourceScopeis incorrectly set to"Subscription".For comparison, another tenant-scoped resource
SupportTicketNoSubCommunicationin the same tspCodeModel.json correctly has"resourceScope": "Tenant".Impact on Generated SDK Code
Due to the incorrect
resourceScope, the generator produces incorrect code forTenantSupportTicketCollection:Wrong
ValidateResourceId: The generated code only validates againstSubscriptionResource.ResourceType, rejectingTenantResource.ResourceType— even though tenant-level access is a valid scenario.Wrong XML doc comments: The generated class doc says:
This is incorrect for a tenant-scoped resource.
Wrong parent scope placement:
GetTenantSupportTickets()is generated onMockableSupportSubscriptionResourceinstead of being also available onMockableSupportTenantResourceas a collection getter.SDK Workarounds Required
These issues required custom code in
TenantSupportTicketCollection.csto:ValidateResourceIdto accept bothSubscriptionResource.ResourceTypeandTenantResource.ResourceTypeGetAll/GetAllAsyncmethods andIEnumerable/IAsyncEnumerableinterfaces for backward compatibilityExpected Behavior
The
TenantSupportTicketresource entry intspCodeModel.jsonshould have:This would cause the generator to:
ValidateResourceIdchecking againstTenantResource.ResourceTypeTenantResourceTenantResourcescopeSpec Reference
49b4c6f0b0496cdbee2be516d45b34aad7bc3fa5specification/support/resource-manager/Microsoft.Support/Support/SupportTicketDetails.tsp💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.