fix(auth-tests): serialize interop test TFMs to fix parallel credential-vault race (#1047)#1320
Conversation
…credential-vault race NativeCredentialStoreInteropTests round-trips a secret through the real per-user OS credential vault. The .NET 9+ SDK runs `dotnet test` on this multi-TFM project (net8.0;net9.0;net10.0) with all frameworks in parallel by default, so three host processes concurrently mutate and enumerate the same vault. Get/Remove enumerate the entire vault (CredEnumerate) before filtering, so a CredRead can observe null while another TFM's CredWrite is in flight — Windows_RoundTripsSecret then fails "expected retrieved not to be null" (issue #1047; the Linux libsecret variant races identically). Set TestTfmsInParallel=false on the test project to serialize the per-TFM hosts. Coverage is unchanged: every TFM still runs its platform-native round-trip, just one at a time. Per-appId GUIDs cannot fix this because the contention is at the vault/enumeration level, not on the key. Fixes #1047 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
joshsmithxrm
temporarily deployed
to
test-dataverse
GitHub Actions
Inactive
joshsmithxrm
temporarily deployed
to
test-dataverse
GitHub Actions
Inactive
joshsmithxrm
temporarily deployed
to
test-dataverse
GitHub Actions
Inactive
joshsmithxrm
temporarily deployed
to
test-dataverse
GitHub Actions
Inactive
joshsmithxrm
temporarily deployed
to
test-dataverse
GitHub Actions
Inactive
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe integration test project disables parallel execution across target frameworks and documents that sequential execution prevents races in native credential store interop tests. ChangesTest execution configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request disables parallel execution of target framework (TFM) test hosts in the integration tests project by setting TestTfmsInParallel to false. This change prevents race conditions when multiple TFMs concurrently access and mutate the shared OS credential vault during tests. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
1 participant
Summary
The
Credential Store Interopworkflow fails intermittently on the windows-latest leg (and, less often, ubuntu-latest) whenever a PR touches its path filter (Directory.Packages.props,Directory.Build.props,src/PPDS.Auth/**). This makes unrelated dependency bumps get a spurious red X.This serializes the per-TFM test hosts so the interop round-trip stops racing on the shared OS credential vault.
Fixes #1047Root cause
NativeCredentialStoreInteropTestsround-trips a secret through the real per-user OS credential vault (Windows Credential Manager / macOS Keychain / libsecret). The test project multi-targetsnet8.0;net9.0;net10.0.The .NET 9+ SDK runs
dotnet teston a multi-TFM project with all target frameworks in parallel by default (introduced in .NET 9 Preview 2). This workflow installs the 10.0.x SDK, so it silently opted into that behavior. Three TFM host processes then hit the same per-user vault concurrently.NativeCredentialStore.GetAsync/RemoveAsyncresolve an entry viaWindowsCredentialManager.Get→Enumerate→CredEnumerate(null, AllCredentials)— a full-vault enumeration filtered in-process. ACredRead/enumeration can observenullfor a just-written entry while a concurrentCredWritefrom another TFM host is in flight, soWindows_RoundTripsSecretfails atretrieved.Should().NotBeNull().Evidence from run 29164790261 (windows-latest), where two TFM hosts finished ~milliseconds apart with interleaved output — one passing, one failing at
NativeCredentialStoreInteropTests.cs:53:Not the skip reporting
The
MacOS_/Linux_RoundTripsSecret [SKIP] — Exception of type 'Xunit.SkipException' was thrownlines are a red herring: in the same logs those tests are correctly tallied asSkipped, and TFM hosts that don't hit the race pass green (Passed: 1, Skipped: 2).Xunit.SkippableFactis working. The only red is the parallel-vault race onWindows_RoundTripsSecret— exactly the bug #1047 documents.Why not "namespace the appId per TFM" (issue's option 1)
The appId is already globally unique per test instance (
ppds-interop-test-{Guid.NewGuid():N}). The contention is not a key collision — it's at the vault/enumeration level (CredEnumerateover the whole vault, plus a shared_manifestentry every write updates). Adding the TFM to an already-unique key would not reduce concurrent vault mutations, so it would not fix the race.Fix
Set
<TestTfmsInParallel>false</TestTfmsInParallel>onPPDS.Auth.IntegrationTests.csproj— the documented, first-class opt-out for the SDK's parallel multi-TFM behavior. The three TFM hosts now run one at a time, eliminating concurrent access to the shared vault. This fixes the Windows Credential Manager race and the identical libsecret race noted on the issue.Placing it in the csproj (rather than the workflow command line) makes the constraint travel with the project across CI, local
dotnet test, and IDE runs — so no future invocation can silently reintroduce the race.Coverage is unchanged: every TFM still runs its platform-native round-trip; they just run sequentially.
Verification
dotnet build PPDS.sln— 0 errors.dotnet test PPDS.sln --filter "Category!=Integration"— 0 failures (full unit suite via pre-commit hook).Windows_RoundTripsSecretruns and passes;MacOS_/Linux_RoundTripsSecretreport Skipped, not Failed.dotnet build ... -getProperty:TestTfmsInParallel→false, confirming the property is honored.Scope
Independent of #1305 (plugins-extract); this only changes the interop test project's build property. A separate latent concern — that two concurrent
ppdsprocesses could hit the same full-vault enumeration race in product code — is out of scope for this test-hardening fix.🤖 Generated with Claude Code
Summary by CodeRabbit