Skip to content

fix: initialize model factories before concurrent reuse - #8347

Open
RKS (rksharma-owg) wants to merge 3 commits into
microsoft:mainfrom
rksharma-owg:codex/fix-model-factory-initialization
Open

RKS (rksharma-owg) wants to merge 3 commits into
microsoft:mainfrom
rksharma-owg:codex/fix-model-factory-initialization

Conversation

@rksharma-owg

@rksharma-owg RKS (rksharma-owg) commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6854.

Parallel endpoints can reuse a response model after it has been added to its namespace but before its discriminator factory exists. The backward-compatible C#/Go response path immediately clones that factory, so the lookup intermittently throws Sequence contains no matching element.

Initialize the factory signature alongside the serialization members before publishing the model. Populate discriminator mappings after property discovery as before. No new waits or lifecycle locks are introduced.

Merged current main (b6f532aee) to resolve conflicts, preserving upstream changes and the focused three-file contribution.

Validation of 1ee39484a4268734dc4a8f965a7ba908826b8fb8:

  • Restore, formatting, and build pass. Full solution: 3,036 passed, three existing skips.
  • Exact-commit fork validation: .NET, CodeQL, integration-tests, idempotency-tests, surface-area-tests. The .NET workflow includes the upstream five-OS release matrix and VS Code extension build/test matrix. Existing integration/idempotency suppressions remain unchanged.
  • Controlled C#/Go concurrency regressions and repeated stress generations run on Linux, Windows, and macOS. Assertions cover factory return types, parse-node parameters, model properties, and backward-compatible endpoint aliases.
  • Required fork checks pass. 14 optional integration/idempotency cases failed under the repository’s existing documented suppressions; these are not counted as passing cases.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 03:34

Copilot AI left a comment

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 review overview

🟢 Approval recommended

The narrowly scoped lifecycle change preserves existing mapping behavior and is covered by a deterministic regression test.

Review effort: Balanced
Findings: None

What changed in this PR

Moves model factory initialization before model publication to prevent concurrent C#/Go backward-compatible response generation failures.

Changes:

  • Initializes factory signatures before shared-model reuse.
  • Populates discriminator mappings after property discovery.
  • Adds deterministic concurrency regression coverage and changelog documentation.
File Description
src/​Kiota.Builder/​KiotaBuilder.cs Fixes concurrent factory availability.
tests/​Kiota.Builder.Tests/​KiotaBuilderTests.ModelFactory.cs Tests controlled C#/Go interleaving.
CHANGELOG.md Documents the fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 14:02

Copilot AI left a comment

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 review overview

🟢 Approval recommended

The initialization ordering fixes the reported race and is covered by focused concurrency tests.

Review effort: Balanced
Findings: None

Copilot AI balanced review requested due to automatic review settings October 8, 2026 03:48
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Conflicts have been resolved. A maintainer will take a look shortly.

Copilot AI left a comment

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.

🟢 Approval recommended

The targeted initialization change resolves the race while preserving mapping behavior and includes focused regression coverage.

0 open findings

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@rksharma-owg
RKS (rksharma-owg) marked this pull request as ready for review October 8, 2026 15:48
@rksharma-owg
RKS (rksharma-owg) requested a review from a team as a code owner October 8, 2026 15:48

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Random error during client generation: Sequence contains no matching element

2 participants