Rename requirement IDs from CACH- to Caching-PLT/Lib/OTS pattern and fix race condition in EnsureCachedAsync

[Copilot]

Pull Request

Description

Requirement IDs used the abbreviation CACH- (not a meaningful product name) and Platform (too verbose) as segment names. Renames all IDs to the Caching-<section>-<name> pattern with abbreviated section tokens.

Mapping applied:

Old ID	New ID
CACH-Lib-EnsureCached	Caching-Lib-EnsureCached
CACH-Platform-Windows	Caching-PLT-Windows
CACH-Platform-Linux	Caching-PLT-Linux
CACH-Platform-MacOS	Caching-PLT-MacOS
CACH-Platform-Net8	Caching-PLT-Net8
CACH-Platform-Net9	Caching-PLT-Net9
CACH-Platform-Net10	Caching-PLT-Net10
CACH-Platform-NetStd20	Caching-PLT-NetStd20
CACH-OTS-* (×6)	Caching-OTS-*

Also corrects a stale CACH-REQ-007 reference in NuGetCacheTests.cs to Caching-Lib-EnsureCached.

Also fixes a race condition in EnsureCachedAsync where concurrent test-framework processes (e.g. net481, net8.0, net9.0, net10.0 running in parallel) could all call EnsureCachedAsync for the same package simultaneously. The old Directory.Exists(packagePath) early-return guard could fire while another process was mid-extraction — returning a partially-populated directory before .nuspec or other files had been written. The fix changes the guard to File.Exists(PathHelpers.SafePathCombine(packagePath, ".nupkg.metadata")): NuGet writes .nupkg.metadata as its last extraction step, so its presence is a definitive "fully installed" signal. If the directory exists but the metadata file is absent, execution falls through to GlobalPackagesFolderUtility.AddPackageAsync, which handles concurrent installs safely via cross-process file locking.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Code quality improvement

Related Issues

Pre-Submission Checklist

Build and Test

• Code builds successfully: dotnet build --configuration Release
• All unit tests pass: dotnet test --configuration Release
• Code produces zero warnings

Code Quality

• Code formatting is correct: dotnet format --verify-no-changes
• New code has appropriate XML documentation comments
• Static analyzer warnings have been addressed

Quality Checks

Please run the following checks before submitting:

• Spell checker passes: cspell "**/*.{md,cs}"
• Markdown linter passes: markdownlint "**/*.md"
• YAML linter passes: yamllint .

Testing

• Added unit tests for new functionality
• Updated existing tests if behavior changed
• All tests follow the AAA (Arrange, Act, Assert) pattern
• Test coverage is maintained or improved

Documentation

• Updated README.md (if applicable)
• Updated ARCHITECTURE.md (if applicable)
• Added code examples for new features (if applicable)
• Updated requirements.yaml (if applicable)

Additional Notes

The requirement ID rename is a pure string change — all test linkages are preserved; only the ID strings themselves changed. The race condition fix touches only the early-return guard in NuGetCache.EnsureCachedAsync, using PathHelpers.SafePathCombine (consistent with the rest of the file) to construct the .nupkg.metadata path; no tests, interfaces, or public API signatures are altered.

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

[Copilot]

Pull request overview

Renames requirements IDs in requirements.yaml to a clearer Caching-<section>-<name> scheme and updates the linked test documentation comment to reference the new library requirement ID.

Changes:

• Updated requirements.yaml requirement IDs from the old CACH-* scheme to Caching-Lib/PLT/OTS-*.
• Updated the NuGetCacheTests XML remark to reference the renamed Caching-Lib-EnsureCached requirement.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
test/DemaConsulting.NuGet.Caching.Tests/NuGetCacheTests.cs	Updates the test’s requirement-reference comment to the new ID.
requirements.yaml	Renames all requirement IDs to the new Caching-* naming pattern while preserving the existing test evidence linkages.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Malcolmnixon]

@copilot Can you think of a reason for this failure:

  Failed NuGetCache_EnsureCachedAsync_ReturnsPackageFolder [570 ms]
  Error Message:
   Assert.IsTrue failed. 'condition' expression: 'hasPackageContent'. Expected package folder to contain .nupkg or .nuspec files at: C:\Users\runneradmin\.nuget\packages\demaconsulting.testresults\1.5.0
  Stack Trace:
     at Microsoft.VisualStudio.TestTools.UnitTesting.Assert.ThrowAssertFailed(String assertionName, String message) in /_/src/TestFramework/TestFramework/Assertions/Assert.cs:line 40
   at Microsoft.VisualStudio.TestTools.UnitTesting.Assert.AssertIsTrueInterpolatedStringHandler.ComputeAssertion(String conditionExpression) in /_/src/TestFramework/TestFramework/Assertions/Assert.IsTrue.cs:line 40
   at DemaConsulting.NuGet.Caching.Tests.NuGetCacheTests.<NuGetCache_EnsureCachedAsync_ReturnsPackageFolder>d__4.MoveNext() in D:\a\NuGetCaching\NuGetCaching\test\DemaConsulting.NuGet.Caching.Tests\NuGetCacheTests.cs:line 63
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestMethodInfo.<ExecuteInternalAsync>d__56.MoveNext() in /_/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs:line 408
Results File: D:\a\NuGetCaching\NuGetCaching\artifacts\windows-latest_net481_20260308011603.trx

Failed!  - Failed:     1, Passed:    12, Skipped:     0, Total:    13, Duration: 879 ms - DemaConsulting.NuGet.Caching.Tests.dll (net481)

[Copilot]

@copilot Can you think of a reason for this failure:

  Failed NuGetCache_EnsureCachedAsync_ReturnsPackageFolder [570 ms]

...

Yes, exactly — it's a race condition. When all four target-framework test processes run simultaneously, one process creates the package directory inside GlobalPackagesFolderUtility.AddPackageAsync (NuGet's first step), and another process's Directory.Exists check fires during that window, sees the directory, and returns immediately — before .nuspec or any other file has been written.

The fix in commit ca29aca changes the early-return guard from Directory.Exists(packagePath) to File.Exists(Path.Combine(packagePath, ".nupkg.metadata")). NuGet writes .nupkg.metadata as its last extraction step, so this file's presence is a definitive "fully installed" signal. If the directory exists mid-extraction but .nupkg.metadata isn't there yet, we fall through to AddPackageAsync, which has its own cross-process file locking to handle concurrency safely.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.