Skip to content

Fix crash in ClientProvider.BuildMethods when children list contains duplicates#10355

Draft
Copilot wants to merge 6 commits intomainfrom
copilot/fix-azure-client-operation-group-crash
Draft

Fix crash in ClientProvider.BuildMethods when children list contains duplicates#10355
Copilot wants to merge 6 commits intomainfrom
copilot/fix-azure-client-operation-group-crash

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 13, 2026

After TCGC 0.67.0 consolidated SdkOperationGroup into SdkClient, InputClient.Children can contain duplicate entries. This causes Fields.ToDictionary(f => f.Name.ToVariableName()) in BuildMethods() to throw "An item with the same key has already been added. Key: cachedGroup3" because duplicate sub-clients produce duplicate caching fields.

Changes

  • client-converter.ts: Deduplicate children in the emitter when converting TCGC clients to the code model, using a loop with a Set<SdkClientType> to skip already-visited clients before converting them. Includes a TODO to remove the deduplication once Duplicate Client Children in Client Hierarchy Azure/typespec-azure#4251 is fixed.
  • client-converter.test.ts: Add test with multiple nested @client interfaces that verifies no duplicate children appear in the generated code model when using @client decorators with operation groups

… crash with duplicate children

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/b15af653-c208-49a5-bd0c-c7bef5ab6a6b

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Apr 13, 2026
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/b15af653-c208-49a5-bd0c-c7bef5ab6a6b

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix crash in Azure Client-Operation-Group spec Fix crash in ClientProvider.BuildMethods when children list contains duplicates Apr 13, 2026
Copilot AI requested a review from jorgerangel-msft April 13, 2026 18:22

private IReadOnlyList<ClientProvider> GetSubClients()
{
var seen = new HashSet<ClientProvider>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot instead of having this change in the generator, can we de-dup the children in the emitter so the generated code model has no duplicate entries?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the deduplication from the C# generator to the emitter in 4cd0636. The client-converter.ts now deduplicates children when populating the code model, and the ClientProvider.cs/ClientProviderTests.cs changes have been reverted.

…rator

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/01cbad23-efa3-4bc8-b1cb-9aa2ec7d8f59

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
diagnostics.pipe(fromSdkClient(sdkContext, c, rootApiVersions)),
);
const seen = new Set<InputClient>();
inputClient.children = client.children
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot can we just create a loop and skip processing any client that was already visited? Can we also add TODO to remove this once Azure/typespec-azure#4251 is fixed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 07b25f5. Replaced the .map().filter() chain with a loop that uses a Set<SdkClientType> to skip already-visited clients before converting them. Also added a TODO referencing Azure/typespec-azure#4251.

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/cd61d1eb-8886-47b1-8549-b54a5a329c54

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/fb528503-05e4-4f56-bbb0-51ea4188d6ae

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@jorgerangel-msft jorgerangel-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot rerun Generate.ps1 to ensure all generated test projects are regenerated. Ensure all tests pass as well, including spector tests

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 13, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10355

commit: 10ef6a7

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 13, 2026

@copilot rerun Generate.ps1 to ensure all generated test projects are regenerated. Ensure all tests pass as well, including spector tests

Ran Generate.ps1 without a filter — no regenerated file changes were detected (all generated test projects were already up to date). All tests pass:

  • Emitter tests: 198 passed (23 test files)
  • ClientModel.Tests: 1,302 passed
  • Generator.Tests: 1,313 passed
  • Input.Tests: 114 passed
  • Local.Tests: 37 passed
  • Spector.Tests: 15 passed, 871 skipped (expected — stubbed implementations)

Copilot AI requested a review from jorgerangel-msft April 13, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in Azure Client-Operation-Group Spector Scenario

2 participants